[PATCH 09/27] drm/etnaviv: don't flush workqueue in etnaviv_gpu_wait_obj_inactive

Russell King - ARM Linux linux at armlinux.org.uk
Fri Dec 1 16:59:23 UTC 2017


On Fri, Dec 01, 2017 at 11:36:06AM +0100, Lucas Stach wrote:
> There is no need to synchronize with oustanding retire jobs if the object
> has gone idle. Retire jobs only ever change the object state from active to
> idle, not the other way around.
> 
> The IOVA put race is uncritical, as the GEM_WAIT ioctl itself is holding
> a reference to the GEM object, so the retire worker will not pull the
> object into the CPU domain, which is the thing we are trying to guard
> against with etnaviv_gpu_wait_obj_inactive.

I don't think this makes sense.

The point of GEM_WAIT is to ensure that the driver has completely
finished with the object, including making sure that the memory
(particularly usermem) is not going to get invalidated unexpectedly.

This is a very important property of GEM_WAIT, which, if violated,
leads to malloc() stack corruption in the Xorg DDX driver.

The explanation above seems to suggest that this condition has been
violated already by the etnaviv driver, which will lead to Xorg
segfaults.

The point to this is to ensure that, at this point in the DDX:

        if (bo->is_usermem)
                etna_bo_gem_wait(bo, VIV_WAIT_INDEFINITE);

        drmIoctl(conn->fd, DRM_IOCTL_GEM_CLOSE, &req);

once we reach here, the usermem object is completely free of any
meddling by the etnaviv driver.  This must *not* happen "at some
unknown point in the future".

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up


More information about the etnaviv mailing list