-----Original Message----- From: Beckett, Robert Sent: Saturday, November 29, 2014 0:59 To: Cheng, Yao; intel-gfx@lists.freedesktop.org; dri- devel@lists.freedesktop.org; daniel.vetter@ffwll.ch; Kelley, Sean V; Chehab, John Cc: emil.l.velikov@gmail.com; Jiang, Fei; dh.herrmann@gmail.com; daniel@fooishbar.org Subject: Re: [Intel-gfx] [RFC PATCH v3 1/4] drm/i915: add i915_ved.c to setup bridge for VED
- Threats:
- Due to the restriction in Linux platform device model, user need
+manually
- uninstall ipvr driver before uninstalling i915 module, otherwise
+he might
- run into use-after-free issues after i915 removes the platform device.
Can you go in to more detail on what you consider to be the restriction in the platform device model?
When removing the device via platform_device_unregister, it will call the remove callback of any drivers handling this device (via the bus remove function). It is then up to the driver to ensure no further usage of the device from which it is being removed. This usually involves removing all user input vectors, disabling interrupts involved and flushing/canceling any delayed work. This should prevent any further use of the device by the driver.
The driver's remove function is called in a direct call chain from platform_device_unregister, so by the time it returns, there should be no further chance of accesses.
Bob, thx for your review comments. The symptom is, after "rmmod i915", though drm_drv_release() is also called on the child device "ipvr", I still see the module exist in the system (check it by "lsmod"). This causes issue when I modprobe i915 and ipvr again later. Actually I don't understand why this restriction exists, but accroding to Daniel, grabbing a module refcount for the platform device doesn't work (it would pin the module forever).
+void vlv_teardown_ved(struct drm_device *dev) {
- struct drm_i915_private *dev_priv = dev->dev_private;
you may want to mask the i915 interrupt for the VED block before removing the device.
Thx, will add it.
- vlv_ved_platdev_destroy(dev);
- if (dev_priv->ved.irq >= 0)
irq_free_desc(dev_priv->ved.irq);+}
Generally it is a nicely interfaced child device.