* fix DMA ops layering violations in vmwgfx
@ 2019-01-05 8:01 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2019-01-05 8:01 UTC (permalink / raw)
To: thellstrom; +Cc: iommu, linux-graphics-maintainer, linux-kernel, dri-devel
Hi Thomas,
vmwgfx has been doing some odd checks based on DMA ops which rely
on deep DMA mapping layer internals, and I think the changes in
Linux 4.21 finally broke most of these implicit assumptions.
The real fix is in patch 3, but I think the others are important
to make it clear what is actually going on.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/4] drm/vmwgfx: remove CONFIG_X86 ifdefs
2019-01-05 8:01 ` Christoph Hellwig
@ 2019-01-05 8:01 ` Christoph Hellwig
-1 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2019-01-05 8:01 UTC (permalink / raw)
To: thellstrom; +Cc: linux-graphics-maintainer, dri-devel, iommu, linux-kernel
The driver depends on CONFIG_X86 so these are dead code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 25afb1d594e3..69e325b2d954 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -565,7 +565,6 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
[vmw_dma_alloc_coherent] = "Using coherent TTM pages.",
[vmw_dma_map_populate] = "Keeping DMA mappings.",
[vmw_dma_map_bind] = "Giving up DMA mappings early."};
-#ifdef CONFIG_X86
const struct dma_map_ops *dma_ops = get_dma_ops(dev_priv->dev->dev);
#ifdef CONFIG_INTEL_IOMMU
@@ -607,11 +606,6 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
if (dev_priv->map_mode == vmw_dma_alloc_coherent)
return -EINVAL;
#endif
-
-#else /* CONFIG_X86 */
- dev_priv->map_mode = vmw_dma_map_populate;
-#endif /* CONFIG_X86 */
-
DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
return 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 1/4] drm/vmwgfx: remove CONFIG_X86 ifdefs
@ 2019-01-05 8:01 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2019-01-05 8:01 UTC (permalink / raw)
To: thellstrom; +Cc: iommu, linux-graphics-maintainer, linux-kernel, dri-devel
The driver depends on CONFIG_X86 so these are dead code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 25afb1d594e3..69e325b2d954 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -565,7 +565,6 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
[vmw_dma_alloc_coherent] = "Using coherent TTM pages.",
[vmw_dma_map_populate] = "Keeping DMA mappings.",
[vmw_dma_map_bind] = "Giving up DMA mappings early."};
-#ifdef CONFIG_X86
const struct dma_map_ops *dma_ops = get_dma_ops(dev_priv->dev->dev);
#ifdef CONFIG_INTEL_IOMMU
@@ -607,11 +606,6 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
if (dev_priv->map_mode == vmw_dma_alloc_coherent)
return -EINVAL;
#endif
-
-#else /* CONFIG_X86 */
- dev_priv->map_mode = vmw_dma_map_populate;
-#endif /* CONFIG_X86 */
-
DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
return 0;
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/4] drm/vmwgfx: remove CONFIG_INTEL_IOMMU ifdefs
2019-01-05 8:01 ` Christoph Hellwig
@ 2019-01-05 8:01 ` Christoph Hellwig
-1 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2019-01-05 8:01 UTC (permalink / raw)
To: thellstrom; +Cc: linux-graphics-maintainer, dri-devel, iommu, linux-kernel
intel_iommu_enabled is defined as always false for !CONFIG_INTEL_IOMMU,
so remove the ifdefs around it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 69e325b2d954..236052ad233c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -567,12 +567,10 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
[vmw_dma_map_bind] = "Giving up DMA mappings early."};
const struct dma_map_ops *dma_ops = get_dma_ops(dev_priv->dev->dev);
-#ifdef CONFIG_INTEL_IOMMU
if (intel_iommu_enabled) {
dev_priv->map_mode = vmw_dma_map_populate;
goto out_fixup;
}
-#endif
if (!(vmw_force_iommu || vmw_force_coherent)) {
dev_priv->map_mode = vmw_dma_phys;
@@ -589,9 +587,7 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
dev_priv->map_mode = vmw_dma_map_populate;
#endif
-#ifdef CONFIG_INTEL_IOMMU
out_fixup:
-#endif
if (dev_priv->map_mode == vmw_dma_map_populate &&
vmw_restrict_iommu)
dev_priv->map_mode = vmw_dma_map_bind;
@@ -599,13 +595,6 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
if (vmw_force_coherent)
dev_priv->map_mode = vmw_dma_alloc_coherent;
-#if !defined(CONFIG_SWIOTLB) && !defined(CONFIG_INTEL_IOMMU)
- /*
- * No coherent page pool
- */
- if (dev_priv->map_mode == vmw_dma_alloc_coherent)
- return -EINVAL;
-#endif
DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
return 0;
@@ -619,7 +608,6 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
* With 32-bit we can only handle 32 bit PFNs. Optionally set that
* restriction also for 64-bit systems.
*/
-#ifdef CONFIG_INTEL_IOMMU
static int vmw_dma_masks(struct vmw_private *dev_priv)
{
struct drm_device *dev = dev_priv->dev;
@@ -631,12 +619,6 @@ static int vmw_dma_masks(struct vmw_private *dev_priv)
}
return 0;
}
-#else
-static int vmw_dma_masks(struct vmw_private *dev_priv)
-{
- return 0;
-}
-#endif
static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
{
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/4] drm/vmwgfx: remove CONFIG_INTEL_IOMMU ifdefs
@ 2019-01-05 8:01 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2019-01-05 8:01 UTC (permalink / raw)
To: thellstrom; +Cc: iommu, linux-graphics-maintainer, linux-kernel, dri-devel
intel_iommu_enabled is defined as always false for !CONFIG_INTEL_IOMMU,
so remove the ifdefs around it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 69e325b2d954..236052ad233c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -567,12 +567,10 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
[vmw_dma_map_bind] = "Giving up DMA mappings early."};
const struct dma_map_ops *dma_ops = get_dma_ops(dev_priv->dev->dev);
-#ifdef CONFIG_INTEL_IOMMU
if (intel_iommu_enabled) {
dev_priv->map_mode = vmw_dma_map_populate;
goto out_fixup;
}
-#endif
if (!(vmw_force_iommu || vmw_force_coherent)) {
dev_priv->map_mode = vmw_dma_phys;
@@ -589,9 +587,7 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
dev_priv->map_mode = vmw_dma_map_populate;
#endif
-#ifdef CONFIG_INTEL_IOMMU
out_fixup:
-#endif
if (dev_priv->map_mode == vmw_dma_map_populate &&
vmw_restrict_iommu)
dev_priv->map_mode = vmw_dma_map_bind;
@@ -599,13 +595,6 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
if (vmw_force_coherent)
dev_priv->map_mode = vmw_dma_alloc_coherent;
-#if !defined(CONFIG_SWIOTLB) && !defined(CONFIG_INTEL_IOMMU)
- /*
- * No coherent page pool
- */
- if (dev_priv->map_mode == vmw_dma_alloc_coherent)
- return -EINVAL;
-#endif
DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
return 0;
@@ -619,7 +608,6 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
* With 32-bit we can only handle 32 bit PFNs. Optionally set that
* restriction also for 64-bit systems.
*/
-#ifdef CONFIG_INTEL_IOMMU
static int vmw_dma_masks(struct vmw_private *dev_priv)
{
struct drm_device *dev = dev_priv->dev;
@@ -631,12 +619,6 @@ static int vmw_dma_masks(struct vmw_private *dev_priv)
}
return 0;
}
-#else
-static int vmw_dma_masks(struct vmw_private *dev_priv)
-{
- return 0;
-}
-#endif
static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
{
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] drm/vmwgfx: remove CONFIG_INTEL_IOMMU ifdefs
2019-01-05 8:01 ` Christoph Hellwig
@ 2019-01-08 10:03 ` Thomas Hellstrom
-1 siblings, 0 replies; 22+ messages in thread
From: Thomas Hellstrom @ 2019-01-08 10:03 UTC (permalink / raw)
To: hch; +Cc: dri-devel, Linux-graphics-maintainer, linux-kernel, iommu
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
On Sat, 2019-01-05 at 09:01 +0100, Christoph Hellwig wrote:
> intel_iommu_enabled is defined as always false for
> !CONFIG_INTEL_IOMMU,
> so remove the ifdefs around it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 69e325b2d954..236052ad233c 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -567,12 +567,10 @@ static int vmw_dma_select_mode(struct
> vmw_private *dev_priv)
> [vmw_dma_map_bind] = "Giving up DMA mappings early."};
> const struct dma_map_ops *dma_ops = get_dma_ops(dev_priv->dev-
> >dev);
>
> -#ifdef CONFIG_INTEL_IOMMU
> if (intel_iommu_enabled) {
> dev_priv->map_mode = vmw_dma_map_populate;
> goto out_fixup;
> }
> -#endif
>
> if (!(vmw_force_iommu || vmw_force_coherent)) {
> dev_priv->map_mode = vmw_dma_phys;
> @@ -589,9 +587,7 @@ static int vmw_dma_select_mode(struct vmw_private
> *dev_priv)
> dev_priv->map_mode = vmw_dma_map_populate;
> #endif
>
> -#ifdef CONFIG_INTEL_IOMMU
> out_fixup:
> -#endif
> if (dev_priv->map_mode == vmw_dma_map_populate &&
> vmw_restrict_iommu)
> dev_priv->map_mode = vmw_dma_map_bind;
> @@ -599,13 +595,6 @@ static int vmw_dma_select_mode(struct
> vmw_private *dev_priv)
> if (vmw_force_coherent)
> dev_priv->map_mode = vmw_dma_alloc_coherent;
>
> -#if !defined(CONFIG_SWIOTLB) && !defined(CONFIG_INTEL_IOMMU)
> - /*
> - * No coherent page pool
> - */
> - if (dev_priv->map_mode == vmw_dma_alloc_coherent)
> - return -EINVAL;
> -#endif
> DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
>
> return 0;
> @@ -619,7 +608,6 @@ static int vmw_dma_select_mode(struct vmw_private
> *dev_priv)
> * With 32-bit we can only handle 32 bit PFNs. Optionally set that
> * restriction also for 64-bit systems.
> */
> -#ifdef CONFIG_INTEL_IOMMU
> static int vmw_dma_masks(struct vmw_private *dev_priv)
> {
> struct drm_device *dev = dev_priv->dev;
> @@ -631,12 +619,6 @@ static int vmw_dma_masks(struct vmw_private
> *dev_priv)
> }
> return 0;
> }
> -#else
> -static int vmw_dma_masks(struct vmw_private *dev_priv)
> -{
> - return 0;
> -}
> -#endif
>
> static int vmw_driver_load(struct drm_device *dev, unsigned long
> chipset)
> {
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] drm/vmwgfx: remove CONFIG_INTEL_IOMMU ifdefs
@ 2019-01-08 10:03 ` Thomas Hellstrom
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Hellstrom @ 2019-01-08 10:03 UTC (permalink / raw)
To: hch; +Cc: iommu, Linux-graphics-maintainer, linux-kernel, dri-devel
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
On Sat, 2019-01-05 at 09:01 +0100, Christoph Hellwig wrote:
> intel_iommu_enabled is defined as always false for
> !CONFIG_INTEL_IOMMU,
> so remove the ifdefs around it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 69e325b2d954..236052ad233c 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -567,12 +567,10 @@ static int vmw_dma_select_mode(struct
> vmw_private *dev_priv)
> [vmw_dma_map_bind] = "Giving up DMA mappings early."};
> const struct dma_map_ops *dma_ops = get_dma_ops(dev_priv->dev-
> >dev);
>
> -#ifdef CONFIG_INTEL_IOMMU
> if (intel_iommu_enabled) {
> dev_priv->map_mode = vmw_dma_map_populate;
> goto out_fixup;
> }
> -#endif
>
> if (!(vmw_force_iommu || vmw_force_coherent)) {
> dev_priv->map_mode = vmw_dma_phys;
> @@ -589,9 +587,7 @@ static int vmw_dma_select_mode(struct vmw_private
> *dev_priv)
> dev_priv->map_mode = vmw_dma_map_populate;
> #endif
>
> -#ifdef CONFIG_INTEL_IOMMU
> out_fixup:
> -#endif
> if (dev_priv->map_mode == vmw_dma_map_populate &&
> vmw_restrict_iommu)
> dev_priv->map_mode = vmw_dma_map_bind;
> @@ -599,13 +595,6 @@ static int vmw_dma_select_mode(struct
> vmw_private *dev_priv)
> if (vmw_force_coherent)
> dev_priv->map_mode = vmw_dma_alloc_coherent;
>
> -#if !defined(CONFIG_SWIOTLB) && !defined(CONFIG_INTEL_IOMMU)
> - /*
> - * No coherent page pool
> - */
> - if (dev_priv->map_mode == vmw_dma_alloc_coherent)
> - return -EINVAL;
> -#endif
> DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
>
> return 0;
> @@ -619,7 +608,6 @@ static int vmw_dma_select_mode(struct vmw_private
> *dev_priv)
> * With 32-bit we can only handle 32 bit PFNs. Optionally set that
> * restriction also for 64-bit systems.
> */
> -#ifdef CONFIG_INTEL_IOMMU
> static int vmw_dma_masks(struct vmw_private *dev_priv)
> {
> struct drm_device *dev = dev_priv->dev;
> @@ -631,12 +619,6 @@ static int vmw_dma_masks(struct vmw_private
> *dev_priv)
> }
> return 0;
> }
> -#else
> -static int vmw_dma_masks(struct vmw_private *dev_priv)
> -{
> - return 0;
> -}
> -#endif
>
> static int vmw_driver_load(struct drm_device *dev, unsigned long
> chipset)
> {
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] drm/vmwgfx: remove CONFIG_INTEL_IOMMU ifdefs
@ 2019-01-08 10:55 ` Thomas Hellstrom
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Hellstrom @ 2019-01-08 10:55 UTC (permalink / raw)
To: hch; +Cc: dri-devel, Linux-graphics-maintainer, linux-kernel, iommu
Hi,
On Sat, 2019-01-05 at 09:01 +0100, Christoph Hellwig wrote:
...
> intel_iommu_enabled is defined as always false for
> !CONFIG_INTEL_IOMMU,
> so remove the ifdefs around it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>
> -#if !defined(CONFIG_SWIOTLB) && !defined(CONFIG_INTEL_IOMMU)
> - /*
> - * No coherent page pool
> - */
> - if (dev_priv->map_mode == vmw_dma_alloc_coherent)
> - return -EINVAL;
> -#endif
>
Actually this hunk is incorrect, it tries to determine whether the TTM
subsystem maintains a coherent page pool or not. If not, we can't use
vmw_dma_alloc_coherent.
/Thomas
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] drm/vmwgfx: remove CONFIG_INTEL_IOMMU ifdefs
@ 2019-01-08 10:55 ` Thomas Hellstrom
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Hellstrom @ 2019-01-08 10:55 UTC (permalink / raw)
To: hch-jcswGhMUV9g
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Linux-graphics-maintainer, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Hi,
On Sat, 2019-01-05 at 09:01 +0100, Christoph Hellwig wrote:
...
> intel_iommu_enabled is defined as always false for
> !CONFIG_INTEL_IOMMU,
> so remove the ifdefs around it.
>
> Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
> ---
>
> -#if !defined(CONFIG_SWIOTLB) && !defined(CONFIG_INTEL_IOMMU)
> - /*
> - * No coherent page pool
> - */
> - if (dev_priv->map_mode == vmw_dma_alloc_coherent)
> - return -EINVAL;
> -#endif
>
Actually this hunk is incorrect, it tries to determine whether the TTM
subsystem maintains a coherent page pool or not. If not, we can't use
vmw_dma_alloc_coherent.
/Thomas
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/4] drm/vmwgfx: fix the check when to use dma_alloc_coherent
2019-01-05 8:01 ` Christoph Hellwig
@ 2019-01-05 8:01 ` Christoph Hellwig
-1 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2019-01-05 8:01 UTC (permalink / raw)
To: thellstrom; +Cc: linux-graphics-maintainer, dri-devel, iommu, linux-kernel
Since Linux 4.21 we merged the swiotlb ops into the DMA direct ops,
so they would always have a the sync_single methods. But late in
the cicle we also removed the direct ops entirely, so we'd see NULL
DMA ops. Switch vmw_dma_select_mode to only detect swiotlb presence
using swiotlb_nr_tbl() instead.
Fixes: 55897af630 ("dma-direct: merge swiotlb_dma_ops into the dma_direct code")
Fixes: 356da6d0cd ("dma-mapping: bypass indirect calls for dma-direct")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 236052ad233c..c2060f6cc9e8 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -565,7 +565,6 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
[vmw_dma_alloc_coherent] = "Using coherent TTM pages.",
[vmw_dma_map_populate] = "Keeping DMA mappings.",
[vmw_dma_map_bind] = "Giving up DMA mappings early."};
- const struct dma_map_ops *dma_ops = get_dma_ops(dev_priv->dev->dev);
if (intel_iommu_enabled) {
dev_priv->map_mode = vmw_dma_map_populate;
@@ -578,14 +577,12 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
return 0;
}
- dev_priv->map_mode = vmw_dma_map_populate;
-
- if (dma_ops && dma_ops->sync_single_for_cpu)
- dev_priv->map_mode = vmw_dma_alloc_coherent;
#ifdef CONFIG_SWIOTLB
- if (swiotlb_nr_tbl() == 0)
- dev_priv->map_mode = vmw_dma_map_populate;
+ if (swiotlb_nr_tbl())
+ dev_priv->map_mode = vmw_dma_alloc_coherent;
+ else
#endif
+ dev_priv->map_mode = vmw_dma_map_populate;
out_fixup:
if (dev_priv->map_mode == vmw_dma_map_populate &&
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] drm/vmwgfx: fix the check when to use dma_alloc_coherent
@ 2019-01-05 8:01 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2019-01-05 8:01 UTC (permalink / raw)
To: thellstrom; +Cc: iommu, linux-graphics-maintainer, linux-kernel, dri-devel
Since Linux 4.21 we merged the swiotlb ops into the DMA direct ops,
so they would always have a the sync_single methods. But late in
the cicle we also removed the direct ops entirely, so we'd see NULL
DMA ops. Switch vmw_dma_select_mode to only detect swiotlb presence
using swiotlb_nr_tbl() instead.
Fixes: 55897af630 ("dma-direct: merge swiotlb_dma_ops into the dma_direct code")
Fixes: 356da6d0cd ("dma-mapping: bypass indirect calls for dma-direct")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 236052ad233c..c2060f6cc9e8 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -565,7 +565,6 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
[vmw_dma_alloc_coherent] = "Using coherent TTM pages.",
[vmw_dma_map_populate] = "Keeping DMA mappings.",
[vmw_dma_map_bind] = "Giving up DMA mappings early."};
- const struct dma_map_ops *dma_ops = get_dma_ops(dev_priv->dev->dev);
if (intel_iommu_enabled) {
dev_priv->map_mode = vmw_dma_map_populate;
@@ -578,14 +577,12 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
return 0;
}
- dev_priv->map_mode = vmw_dma_map_populate;
-
- if (dma_ops && dma_ops->sync_single_for_cpu)
- dev_priv->map_mode = vmw_dma_alloc_coherent;
#ifdef CONFIG_SWIOTLB
- if (swiotlb_nr_tbl() == 0)
- dev_priv->map_mode = vmw_dma_map_populate;
+ if (swiotlb_nr_tbl())
+ dev_priv->map_mode = vmw_dma_alloc_coherent;
+ else
#endif
+ dev_priv->map_mode = vmw_dma_map_populate;
out_fixup:
if (dev_priv->map_mode == vmw_dma_map_populate &&
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/4] drm/vmwgfx: unwind spaghetti code in vmw_dma_select_mode
2019-01-05 8:01 ` Christoph Hellwig
@ 2019-01-05 8:01 ` Christoph Hellwig
-1 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2019-01-05 8:01 UTC (permalink / raw)
To: thellstrom; +Cc: linux-graphics-maintainer, dri-devel, iommu, linux-kernel
Just use a simple if/else chain to select the DMA mode.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 25 ++++++-------------------
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index c2060f6cc9e8..86387735a90b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -566,34 +566,21 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
[vmw_dma_map_populate] = "Keeping DMA mappings.",
[vmw_dma_map_bind] = "Giving up DMA mappings early."};
- if (intel_iommu_enabled) {
+ if (vmw_force_coherent)
+ dev_priv->map_mode = vmw_dma_alloc_coherent;
+ else if (intel_iommu_enabled)
dev_priv->map_mode = vmw_dma_map_populate;
- goto out_fixup;
- }
-
- if (!(vmw_force_iommu || vmw_force_coherent)) {
+ else if (!vmw_force_iommu)
dev_priv->map_mode = vmw_dma_phys;
- DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
- return 0;
- }
-
-#ifdef CONFIG_SWIOTLB
- if (swiotlb_nr_tbl())
+ else if (IS_ENABLED(CONFIG_SWIOTLB) && swiotlb_nr_tbl())
dev_priv->map_mode = vmw_dma_alloc_coherent;
else
-#endif
dev_priv->map_mode = vmw_dma_map_populate;
-out_fixup:
- if (dev_priv->map_mode == vmw_dma_map_populate &&
- vmw_restrict_iommu)
+ if (dev_priv->map_mode == vmw_dma_map_populate && vmw_restrict_iommu)
dev_priv->map_mode = vmw_dma_map_bind;
- if (vmw_force_coherent)
- dev_priv->map_mode = vmw_dma_alloc_coherent;
-
DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
-
return 0;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/4] drm/vmwgfx: unwind spaghetti code in vmw_dma_select_mode
@ 2019-01-05 8:01 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2019-01-05 8:01 UTC (permalink / raw)
To: thellstrom; +Cc: iommu, linux-graphics-maintainer, linux-kernel, dri-devel
Just use a simple if/else chain to select the DMA mode.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 25 ++++++-------------------
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index c2060f6cc9e8..86387735a90b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -566,34 +566,21 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
[vmw_dma_map_populate] = "Keeping DMA mappings.",
[vmw_dma_map_bind] = "Giving up DMA mappings early."};
- if (intel_iommu_enabled) {
+ if (vmw_force_coherent)
+ dev_priv->map_mode = vmw_dma_alloc_coherent;
+ else if (intel_iommu_enabled)
dev_priv->map_mode = vmw_dma_map_populate;
- goto out_fixup;
- }
-
- if (!(vmw_force_iommu || vmw_force_coherent)) {
+ else if (!vmw_force_iommu)
dev_priv->map_mode = vmw_dma_phys;
- DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
- return 0;
- }
-
-#ifdef CONFIG_SWIOTLB
- if (swiotlb_nr_tbl())
+ else if (IS_ENABLED(CONFIG_SWIOTLB) && swiotlb_nr_tbl())
dev_priv->map_mode = vmw_dma_alloc_coherent;
else
-#endif
dev_priv->map_mode = vmw_dma_map_populate;
-out_fixup:
- if (dev_priv->map_mode == vmw_dma_map_populate &&
- vmw_restrict_iommu)
+ if (dev_priv->map_mode == vmw_dma_map_populate && vmw_restrict_iommu)
dev_priv->map_mode = vmw_dma_map_bind;
- if (vmw_force_coherent)
- dev_priv->map_mode = vmw_dma_alloc_coherent;
-
DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
-
return 0;
}
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 4/4] drm/vmwgfx: unwind spaghetti code in vmw_dma_select_mode
@ 2019-01-08 10:56 ` Thomas Hellstrom
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Hellstrom @ 2019-01-08 10:56 UTC (permalink / raw)
To: hch; +Cc: dri-devel, Linux-graphics-maintainer, linux-kernel, iommu
On Sat, 2019-01-05 at 09:01 +0100, Christoph Hellwig wrote:
> Just use a simple if/else chain to select the DMA mode.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 25 ++++++-------------------
> 1 file changed, 6 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index c2060f6cc9e8..86387735a90b 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -566,34 +566,21 @@ static int vmw_dma_select_mode(struct
> vmw_private *dev_priv)
> [vmw_dma_map_populate] = "Keeping DMA mappings.",
> [vmw_dma_map_bind] = "Giving up DMA mappings early."};
>
> - if (intel_iommu_enabled) {
> + if (vmw_force_coherent)
> + dev_priv->map_mode = vmw_dma_alloc_coherent;
> + else if (intel_iommu_enabled)
> dev_priv->map_mode = vmw_dma_map_populate;
> - goto out_fixup;
> - }
> -
> - if (!(vmw_force_iommu || vmw_force_coherent)) {
> + else if (!vmw_force_iommu)
> dev_priv->map_mode = vmw_dma_phys;
> - DRM_INFO("DMA map mode: %s\n", names[dev_priv-
> >map_mode]);
> - return 0;
> - }
> -
> -#ifdef CONFIG_SWIOTLB
> - if (swiotlb_nr_tbl())
> + else if (IS_ENABLED(CONFIG_SWIOTLB) && swiotlb_nr_tbl())
> dev_priv->map_mode = vmw_dma_alloc_coherent;
> else
> -#endif
> dev_priv->map_mode = vmw_dma_map_populate;
>
> -out_fixup:
> - if (dev_priv->map_mode == vmw_dma_map_populate &&
> - vmw_restrict_iommu)
> + if (dev_priv->map_mode == vmw_dma_map_populate &&
> vmw_restrict_iommu)
> dev_priv->map_mode = vmw_dma_map_bind;
>
> - if (vmw_force_coherent)
> - dev_priv->map_mode = vmw_dma_alloc_coherent;
> -
> DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
> -
> return 0;
> }
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/4] drm/vmwgfx: unwind spaghetti code in vmw_dma_select_mode
@ 2019-01-08 10:56 ` Thomas Hellstrom
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Hellstrom @ 2019-01-08 10:56 UTC (permalink / raw)
To: hch-jcswGhMUV9g
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Linux-graphics-maintainer, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Sat, 2019-01-05 at 09:01 +0100, Christoph Hellwig wrote:
> Just use a simple if/else chain to select the DMA mode.
>
> Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Reviewed-by: Thomas Hellstrom <thellstrom-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 25 ++++++-------------------
> 1 file changed, 6 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index c2060f6cc9e8..86387735a90b 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -566,34 +566,21 @@ static int vmw_dma_select_mode(struct
> vmw_private *dev_priv)
> [vmw_dma_map_populate] = "Keeping DMA mappings.",
> [vmw_dma_map_bind] = "Giving up DMA mappings early."};
>
> - if (intel_iommu_enabled) {
> + if (vmw_force_coherent)
> + dev_priv->map_mode = vmw_dma_alloc_coherent;
> + else if (intel_iommu_enabled)
> dev_priv->map_mode = vmw_dma_map_populate;
> - goto out_fixup;
> - }
> -
> - if (!(vmw_force_iommu || vmw_force_coherent)) {
> + else if (!vmw_force_iommu)
> dev_priv->map_mode = vmw_dma_phys;
> - DRM_INFO("DMA map mode: %s\n", names[dev_priv-
> >map_mode]);
> - return 0;
> - }
> -
> -#ifdef CONFIG_SWIOTLB
> - if (swiotlb_nr_tbl())
> + else if (IS_ENABLED(CONFIG_SWIOTLB) && swiotlb_nr_tbl())
> dev_priv->map_mode = vmw_dma_alloc_coherent;
> else
> -#endif
> dev_priv->map_mode = vmw_dma_map_populate;
>
> -out_fixup:
> - if (dev_priv->map_mode == vmw_dma_map_populate &&
> - vmw_restrict_iommu)
> + if (dev_priv->map_mode == vmw_dma_map_populate &&
> vmw_restrict_iommu)
> dev_priv->map_mode = vmw_dma_map_bind;
>
> - if (vmw_force_coherent)
> - dev_priv->map_mode = vmw_dma_alloc_coherent;
> -
> DRM_INFO("DMA map mode: %s\n", names[dev_priv->map_mode]);
> -
> return 0;
> }
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix DMA ops layering violations in vmwgfx
2019-01-05 8:01 ` Christoph Hellwig
` (4 preceding siblings ...)
(?)
@ 2019-01-08 9:51 ` Thomas Hellstrom
2019-01-08 13:12 ` hch-jcswGhMUV9g
-1 siblings, 1 reply; 22+ messages in thread
From: Thomas Hellstrom @ 2019-01-08 9:51 UTC (permalink / raw)
To: hch; +Cc: dri-devel, Linux-graphics-maintainer, linux-kernel, iommu
Hi, Christoph,
On Sat, 2019-01-05 at 09:01 +0100, Christoph Hellwig wrote:
> Hi Thomas,
>
> vmwgfx has been doing some odd checks based on DMA ops which rely
> on deep DMA mapping layer internals, and I think the changes in
> Linux 4.21 finally broke most of these implicit assumptions.
Thanks.
What we're really trying to do here is to try to detect the situation
where DMA remapping using hardware IOMMUs is going on but memory is
still coherent, since the driver can currently only work with coherent
memory[1]. Currently we use intel_iommu_enabled to detect this
situation, but it would be really helpful if there were a generic bool
that advertizes this situation since we need to deal with other IOMMUs
as well going forward. Any suggestion?
Comments on the patches separately.
Thanks,
Thomas
>
> The real fix is in patch 3, but I think the others are important
> to make it clear what is actually going on.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix DMA ops layering violations in vmwgfx
@ 2019-01-08 13:12 ` hch-jcswGhMUV9g
0 siblings, 0 replies; 22+ messages in thread
From: hch @ 2019-01-08 13:12 UTC (permalink / raw)
To: Thomas Hellstrom
Cc: hch, dri-devel, Linux-graphics-maintainer, linux-kernel, iommu
On Tue, Jan 08, 2019 at 09:51:45AM +0000, Thomas Hellstrom wrote:
> Hi, Christoph,
>
> On Sat, 2019-01-05 at 09:01 +0100, Christoph Hellwig wrote:
> > Hi Thomas,
> >
> > vmwgfx has been doing some odd checks based on DMA ops which rely
> > on deep DMA mapping layer internals, and I think the changes in
> > Linux 4.21 finally broke most of these implicit assumptions.
>
> Thanks.
> What we're really trying to do here is to try to detect the situation
> where DMA remapping using hardware IOMMUs is going on but memory is
> still coherent, since the driver can currently only work with coherent
> memory[1]. Currently we use intel_iommu_enabled to detect this
> situation, but it would be really helpful if there were a generic bool
> that advertizes this situation since we need to deal with other IOMMUs
> as well going forward. Any suggestion?
I'm missing the link of the [1] reference above. But if you need
coherent memory you should simply always use dma_alloc_coherent, that
is the only gurantee you get. If you use any other dma mapping methods
you will otherwise need to explicitly transfer ownership by mapping/
unmapping or using dma_sync* before and after every device access.
And the whole DMA API bypass using the phys mode is something that
I'd really prefer not to see in any driver as it tends to cause
major problems sooner or later.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix DMA ops layering violations in vmwgfx
@ 2019-01-08 13:12 ` hch-jcswGhMUV9g
0 siblings, 0 replies; 22+ messages in thread
From: hch-jcswGhMUV9g @ 2019-01-08 13:12 UTC (permalink / raw)
To: Thomas Hellstrom
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Linux-graphics-maintainer, hch-jcswGhMUV9g,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Tue, Jan 08, 2019 at 09:51:45AM +0000, Thomas Hellstrom wrote:
> Hi, Christoph,
>
> On Sat, 2019-01-05 at 09:01 +0100, Christoph Hellwig wrote:
> > Hi Thomas,
> >
> > vmwgfx has been doing some odd checks based on DMA ops which rely
> > on deep DMA mapping layer internals, and I think the changes in
> > Linux 4.21 finally broke most of these implicit assumptions.
>
> Thanks.
> What we're really trying to do here is to try to detect the situation
> where DMA remapping using hardware IOMMUs is going on but memory is
> still coherent, since the driver can currently only work with coherent
> memory[1]. Currently we use intel_iommu_enabled to detect this
> situation, but it would be really helpful if there were a generic bool
> that advertizes this situation since we need to deal with other IOMMUs
> as well going forward. Any suggestion?
I'm missing the link of the [1] reference above. But if you need
coherent memory you should simply always use dma_alloc_coherent, that
is the only gurantee you get. If you use any other dma mapping methods
you will otherwise need to explicitly transfer ownership by mapping/
unmapping or using dma_sync* before and after every device access.
And the whole DMA API bypass using the phys mode is something that
I'd really prefer not to see in any driver as it tends to cause
major problems sooner or later.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix DMA ops layering violations in vmwgfx
@ 2019-01-08 18:25 ` Thomas Hellstrom
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Hellstrom @ 2019-01-08 18:25 UTC (permalink / raw)
To: hch; +Cc: dri-devel, Linux-graphics-maintainer, linux-kernel, iommu
On Tue, 2019-01-08 at 14:12 +0100, hch@lst.de wrote:
> On Tue, Jan 08, 2019 at 09:51:45AM +0000, Thomas Hellstrom wrote:
> > Hi, Christoph,
> >
> > On Sat, 2019-01-05 at 09:01 +0100, Christoph Hellwig wrote:
> > > Hi Thomas,
> > >
> > > vmwgfx has been doing some odd checks based on DMA ops which rely
> > > on deep DMA mapping layer internals, and I think the changes in
> > > Linux 4.21 finally broke most of these implicit assumptions.
> >
> > Thanks.
> > What we're really trying to do here is to try to detect the
> > situation
> > where DMA remapping using hardware IOMMUs is going on but memory is
> > still coherent, since the driver can currently only work with
> > coherent
> > memory[1]. Currently we use intel_iommu_enabled to detect this
> > situation, but it would be really helpful if there were a generic
> > bool
> > that advertizes this situation since we need to deal with other
> > IOMMUs
> > as well going forward. Any suggestion?
>
> I'm missing the link of the [1] reference above.
Sorry. I meant to remove the reference since the explanation would be
rather lengthy.
> But if you need
> coherent memory you should simply always use dma_alloc_coherent, that
> is the only gurantee you get. If you use any other dma mapping
> methods
> you will otherwise need to explicitly transfer ownership by mapping/
> unmapping or using dma_sync* before and after every device access.
The problem is that graphics applications typically allocate huge
amounts of DMA memory. The GPU may want to map half of available system
memory or more. Moreover this memory might need to be mappable by
multiple devices, which would rule out dma_alloc_coherent() in many
situations.
Using SWIOTLB with bounce-buffers is also typically out of the question
for performance- and memory usage reasons. Moreover the sync operations
would often affect sub-regions of 2D and 3D textures, which makes the
dma_sync API inefficient even if it maps to no-ops. vmwgfx would rather
not load in these situations.
Finally graphics applications, do not always provide conservative sync
regions.
>
> And the whole DMA API bypass using the phys mode is something that
> I'd really prefer not to see in any driver as it tends to cause
> major problems sooner or later.
I fully understand this. However, given the above, the DMA API is quite
awkward for graphics operation. It would be easier to motivate a full
removal of the phys mode if we could query the DMA API whether the
dma_sync operations are no-ops or not.
/Thomas
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: fix DMA ops layering violations in vmwgfx
@ 2019-01-08 18:25 ` Thomas Hellstrom
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Hellstrom @ 2019-01-08 18:25 UTC (permalink / raw)
To: hch-jcswGhMUV9g
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Linux-graphics-maintainer, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Tue, 2019-01-08 at 14:12 +0100, hch-jcswGhMUV9g@public.gmane.org wrote:
> On Tue, Jan 08, 2019 at 09:51:45AM +0000, Thomas Hellstrom wrote:
> > Hi, Christoph,
> >
> > On Sat, 2019-01-05 at 09:01 +0100, Christoph Hellwig wrote:
> > > Hi Thomas,
> > >
> > > vmwgfx has been doing some odd checks based on DMA ops which rely
> > > on deep DMA mapping layer internals, and I think the changes in
> > > Linux 4.21 finally broke most of these implicit assumptions.
> >
> > Thanks.
> > What we're really trying to do here is to try to detect the
> > situation
> > where DMA remapping using hardware IOMMUs is going on but memory is
> > still coherent, since the driver can currently only work with
> > coherent
> > memory[1]. Currently we use intel_iommu_enabled to detect this
> > situation, but it would be really helpful if there were a generic
> > bool
> > that advertizes this situation since we need to deal with other
> > IOMMUs
> > as well going forward. Any suggestion?
>
> I'm missing the link of the [1] reference above.
Sorry. I meant to remove the reference since the explanation would be
rather lengthy.
> But if you need
> coherent memory you should simply always use dma_alloc_coherent, that
> is the only gurantee you get. If you use any other dma mapping
> methods
> you will otherwise need to explicitly transfer ownership by mapping/
> unmapping or using dma_sync* before and after every device access.
The problem is that graphics applications typically allocate huge
amounts of DMA memory. The GPU may want to map half of available system
memory or more. Moreover this memory might need to be mappable by
multiple devices, which would rule out dma_alloc_coherent() in many
situations.
Using SWIOTLB with bounce-buffers is also typically out of the question
for performance- and memory usage reasons. Moreover the sync operations
would often affect sub-regions of 2D and 3D textures, which makes the
dma_sync API inefficient even if it maps to no-ops. vmwgfx would rather
not load in these situations.
Finally graphics applications, do not always provide conservative sync
regions.
>
> And the whole DMA API bypass using the phys mode is something that
> I'd really prefer not to see in any driver as it tends to cause
> major problems sooner or later.
I fully understand this. However, given the above, the DMA API is quite
awkward for graphics operation. It would be easier to motivate a full
removal of the phys mode if we could query the DMA API whether the
dma_sync operations are no-ops or not.
/Thomas
^ permalink raw reply [flat|nested] 22+ messages in thread