[PATCH] drm/vmwgfx: fix a warning due to missing dma_parms

[PATCH] drm/vmwgfx: fix a warning due to missing dma_parms

[Qian Cai]

Booting up with DMA_API_DEBUG_SG=y generates a warning below due to the
driver forgot to set dma_parms appropriately. Set it after
vmw_dma_masks(), so it can choose a size either DMA_BIT_MASK(64) or
DMA_BIT_MASK(44).

DMA-API: vmwgfx 0000:00:0f.0: mapping sg segment longer than device
claims to support [len=2097152] [max=65536]
WARNING: CPU: 2 PID: 261 at kernel/dma/debug.c:1232
debug_dma_map_sg+0x360/0x480
Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop
Reference Platform, BIOS 6.00 04/13/2018
RIP: 0010:debug_dma_map_sg+0x360/0x480
Call Trace:
 vmw_ttm_map_dma+0x3b1/0x5b0 [vmwgfx]
 vmw_bo_map_dma+0x25/0x30 [vmwgfx]
 vmw_otables_setup+0x2a8/0x750 [vmwgfx]
 vmw_request_device_late+0x78/0xc0 [vmwgfx]
 vmw_request_device+0xee/0x4e0 [vmwgfx]
 vmw_driver_load.cold+0x757/0xd84 [vmwgfx]
 drm_dev_register+0x1ff/0x340 [drm]
 drm_get_pci_dev+0x110/0x290 [drm]
 vmw_probe+0x15/0x20 [vmwgfx]
 local_pci_probe+0x7a/0xc0
 pci_device_probe+0x1b9/0x290
 really_probe+0x1b5/0x630
 driver_probe_device+0xa3/0x1a0
 device_driver_attach+0x94/0xa0
 __driver_attach+0xdd/0x1c0
 bus_for_each_dev+0xfe/0x150
 driver_attach+0x2d/0x40
 bus_add_driver+0x290/0x350
 driver_register+0xdc/0x1d0
 __pci_register_driver+0xda/0xf0
 vmwgfx_init+0x34/0x1000 [vmwgfx]
 do_one_initcall+0xe5/0x40a
 do_init_module+0x10f/0x3a0
 load_module+0x16a5/0x1a40
 __se_sys_finit_module+0x183/0x1c0
 __x64_sys_finit_module+0x43/0x50
 do_syscall_64+0xc8/0x606
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Fixes: fb1d9738ca05 ("drm/vmwgfx: Add DRM driver for VMware Virtual GPU")
Signed-off-by: Qian Cai <cai@lca.pw>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index bf6c3500d363..5c567b81174f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -747,6 +747,13 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
 	if (unlikely(ret != 0))
 		goto out_err0;
 
+	dev->dev->dma_parms =  kzalloc(sizeof(*dev->dev->dma_parms),
+				       GFP_KERNEL);
+	if (!dev->dev->dma_parms)
+		goto out_err0;
+
+	dma_set_max_seg_size(dev->dev, *dev->dev->dma_mask);
+
 	if (dev_priv->capabilities & SVGA_CAP_GMR2) {
 		DRM_INFO("Max GMR ids is %u\n",
 			 (unsigned)dev_priv->max_gmr_ids);
@@ -772,7 +779,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
 	if (unlikely(dev_priv->mmio_virt == NULL)) {
 		ret = -ENOMEM;
 		DRM_ERROR("Failed mapping MMIO.\n");
-		goto out_err0;
+		goto out_err1;
 	}
 
 	/* Need mmio memory to check for fifo pitchlock cap. */
@@ -781,7 +788,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
 	    !vmw_fifo_have_pitchlock(dev_priv)) {
 		ret = -ENOSYS;
 		DRM_ERROR("Hardware has no pitchlock\n");
-		goto out_err4;
+		goto out_err2;
 	}
 
 	dev_priv->tdev = ttm_object_device_init(&ttm_mem_glob, 12,
@@ -790,7 +797,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
 	if (unlikely(dev_priv->tdev == NULL)) {
 		DRM_ERROR("Unable to initialize TTM object management.\n");
 		ret = -ENOMEM;
-		goto out_err4;
+		goto out_err2;
 	}
 
 	dev->dev_private = dev_priv;
@@ -944,8 +951,11 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
 		pci_release_regions(dev->pdev);
 out_no_device:
 	ttm_object_device_release(&dev_priv->tdev);
-out_err4:
+out_err2:
 	memunmap(dev_priv->mmio_virt);
+out_err1:
+	kfree(dev->dev->dma_parms);
+	dev->dev->dma_parms = NULL;
 out_err0:
 	for (i = vmw_res_context; i < vmw_res_max; ++i)
 		idr_destroy(&dev_priv->res_idr[i]);
@@ -995,6 +1005,8 @@ static void vmw_driver_unload(struct drm_device *dev)
 
 	ttm_object_device_release(&dev_priv->tdev);
 	memunmap(dev_priv->mmio_virt);
+	kfree(dev->dev->dma_parms);
+	dev->dev->dma_parms = NULL;
 	if (dev_priv->ctx.staged_bindings)
 		vmw_binding_state_free(dev_priv->ctx.staged_bindings);
 
-- 
2.20.1 (Apple Git-117)

Re: [PATCH] drm/vmwgfx: fix a warning due to missing dma_parms

[Christoph Hellwig]

On Thu, May 23, 2019 at 10:37:19PM -0400, Qian Cai wrote:
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index bf6c3500d363..5c567b81174f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -747,6 +747,13 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
>  	if (unlikely(ret != 0))
>  		goto out_err0;
>  
> +	dev->dev->dma_parms =  kzalloc(sizeof(*dev->dev->dma_parms),
> +				       GFP_KERNEL);
> +	if (!dev->dev->dma_parms)
> +		goto out_err0;

What bus does this device come from?  I though vmgfx was a (virtualized)
PCI device, in which case this should be provided by the PCI core.
Or are we calling DMA mapping routines on arbitrary other struct device,
in which case that is the real bug and we should switch the PCI device
instead.

> +	dma_set_max_seg_size(dev->dev, *dev->dev->dma_mask);

That looks odd.  If you want to support an unlimited segment size
just pass UINT_MAX here.

Re: [PATCH] drm/vmwgfx: fix a warning due to missing dma_parms

[Thomas Hellstrom]

On Fri, 2019-05-24 at 08:19 +0200, Christoph Hellwig wrote:
> On Thu, May 23, 2019 at 10:37:19PM -0400, Qian Cai wrote:
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > index bf6c3500d363..5c567b81174f 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > @@ -747,6 +747,13 @@ static int vmw_driver_load(struct drm_device
> > *dev, unsigned long chipset)
> >  	if (unlikely(ret != 0))
> >  		goto out_err0;
> >  
> > +	dev->dev->dma_parms =  kzalloc(sizeof(*dev->dev->dma_parms),
> > +				       GFP_KERNEL);
> > +	if (!dev->dev->dma_parms)
> > +		goto out_err0;
> 
> What bus does this device come from?  I though vmgfx was a
> (virtualized)
> PCI device, in which case this should be provided by the PCI core.
> Or are we calling DMA mapping routines on arbitrary other struct
> device,
> in which case that is the real bug and we should switch the PCI
> device
> instead.

It's a PCI device. The struct device * used in dma_map_sg() is the same
as the &pci_dev->dev handed to the probe() callback. But at probe time,
the struct device::dma_parms is non-NULL, at least on my system so
there shouldn't really be a need to kzalloc() it.

> 
> > +	dma_set_max_seg_size(dev->dev, *dev->dev->dma_mask);

The max is U32_MAX.

/Thomas


> 
> That looks odd.  If you want to support an unlimited segment size
> just pass UINT_MAX here.

Re: [PATCH] drm/vmwgfx: fix a warning due to missing dma_parms

[hch]

On Fri, May 24, 2019 at 11:57:04AM +0000, Thomas Hellstrom wrote:
> It's a PCI device. The struct device * used in dma_map_sg() is the same
> as the &pci_dev->dev handed to the probe() callback. But at probe time,
> the struct device::dma_parms is non-NULL, at least on my system so
> there shouldn't really be a need to kzalloc() it.

Then there is something really odd going on in the OPs setup..