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 <[email protected]>
---
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)
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.
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.
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..