From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f68.google.com ([74.125.82.68]:32831 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752660AbcBHBOm (ORCPT ); Sun, 7 Feb 2016 20:14:42 -0500 Received: by mail-wm0-f68.google.com with SMTP id r129so13228211wmr.0 for ; Sun, 07 Feb 2016 17:14:42 -0800 (PST) From: Mario Kleiner To: dri-devel@lists.freedesktop.org Cc: mario.kleiner.de@gmail.com, linux@bernd-steinhauser.de, , michel@daenzer.net, vbabka@suse.cz, ville.syrjala@linux.intel.com, daniel.vetter@ffwll.ch, alexander.deucher@amd.com, christian.koenig@amd.com Subject: [PATCH 2/6] drm: Prevent vblank counter bumps > 1 with active vblank clients. Date: Mon, 8 Feb 2016 02:13:25 +0100 Message-Id: <1454894009-15466-3-git-send-email-mario.kleiner.de@gmail.com> In-Reply-To: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> References: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> Sender: stable-owner@vger.kernel.org List-ID: This fixes a regression introduced by the new drm_update_vblank_count() implementation in Linux 4.4: Restrict the bump of the software vblank counter in drm_update_vblank_count() to a safe maximum value of +1 whenever there is the possibility that concurrent readers of vblank timestamps could be active at the moment, as the current implementation of the timestamp caching and updating is not safe against concurrent readers for calls to store_vblank() with a bump of anything but +1. A bump != 1 would very likely return corrupted timestamps to userspace, because the same slot in the cache could be concurrently written by store_vblank() and read by one of those readers in a non-atomic fashion and without the read-retry logic detecting this collision. Concurrent readers can exist while drm_update_vblank_count() is called from the drm_vblank_off() or drm_vblank_on() functions or other non-vblank- irq callers. However, all those calls are happening with the vbl_lock locked thereby preventing a drm_vblank_get(), so the vblank refcount can't increase while drm_update_vblank_count() is executing. Therefore a zero vblank refcount during execution of that function signals that is safe for arbitrary counter bumps if called from outside vblank irq, whereas a non-zero count is not safe. Whenever the function is called from vblank irq, we have to assume concurrent readers could show up any time during its execution, even if the refcount is currently zero, as vblank irqs are usually only enabled due to the presence of readers, and because when it is called from vblank irq it can't hold the vbl_lock to protect it from sudden bumps in vblank refcount. Therefore also restrict bumps to +1 when the function is called from vblank irq. Such bumps of more than +1 can happen at other times than reenabling vblank irqs, e.g., when regular vblank interrupts get delayed by more than 1 frame due to long held locks, long irq off periods, realtime preemption on RT kernels, or system management interrupts. Signed-off-by: Mario Kleiner Cc: # 4.4+ Cc: michel@daenzer.net Cc: vbabka@suse.cz Cc: ville.syrjala@linux.intel.com Cc: daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: alexander.deucher@amd.com Cc: christian.koenig@amd.com --- drivers/gpu/drm/drm_irq.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index bcb8528..aa2c74b 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -221,6 +221,47 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, diff = (flags & DRM_CALLED_FROM_VBLIRQ) != 0; } + /* + * Restrict the bump of the software vblank counter to a safe maximum + * value of +1 whenever there is the possibility that concurrent readers + * of vblank timestamps could be active at the moment, as the current + * implementation of the timestamp caching and updating is not safe + * against concurrent readers for calls to store_vblank() with a bump + * of anything but +1. A bump != 1 would very likely return corrupted + * timestamps to userspace, because the same slot in the cache could + * be concurrently written by store_vblank() and read by one of those + * readers without the read-retry logic detecting the collision. + * + * Concurrent readers can exist when we are called from the + * drm_vblank_off() or drm_vblank_on() functions and other non-vblank- + * irq callers. However, all those calls to us are happening with the + * vbl_lock locked to prevent drm_vblank_get(), so the vblank refcount + * can't increase while we are executing. Therefore a zero refcount at + * this point is safe for arbitrary counter bumps if we are called + * outside vblank irq, a non-zero count is not 100% safe. Unfortunately + * we must also accept a refcount of 1, as whenever we are called from + * drm_vblank_get() -> drm_vblank_enable() the refcount will be 1 and + * we must let that one pass through in order to not lose vblank counts + * during vblank irq off - which would completely defeat the whole + * point of this routine. + * + * Whenever we are called from vblank irq, we have to assume concurrent + * readers exist or can show up any time during our execution, even if + * the refcount is currently zero, as vblank irqs are usually only + * enabled due to the presence of readers, and because when we are called + * from vblank irq we can't hold the vbl_lock to protect us from sudden + * bumps in vblank refcount. Therefore also restrict bumps to +1 when + * called from vblank irq. + */ + if ((diff > 1) && (atomic_read(&vblank->refcount) > 1 || + (flags & DRM_CALLED_FROM_VBLIRQ))) { + DRM_DEBUG_VBL("clamping vblank bump to 1 on crtc %u: diffr=%u " + "refcount %u, vblirq %u\n", pipe, diff, + atomic_read(&vblank->refcount), + (flags & DRM_CALLED_FROM_VBLIRQ) != 0); + diff = 1; + } + DRM_DEBUG_VBL("updating vblank count on crtc %u:" " current=%u, diff=%u, hw=%u hw_last=%u\n", pipe, vblank->count, diff, cur_vblank, vblank->last); -- 1.9.1 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mario Kleiner Subject: [PATCH 2/6] drm: Prevent vblank counter bumps > 1 with active vblank clients. Date: Mon, 8 Feb 2016 02:13:25 +0100 Message-ID: <1454894009-15466-3-git-send-email-mario.kleiner.de@gmail.com> References: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id DAE5E6E328 for ; Sun, 7 Feb 2016 17:14:42 -0800 (PST) Received: by mail-wm0-f65.google.com with SMTP id r129so13228212wmr.0 for ; Sun, 07 Feb 2016 17:14:42 -0800 (PST) In-Reply-To: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: dri-devel@lists.freedesktop.org Cc: daniel.vetter@ffwll.ch, michel@daenzer.net, linux@bernd-steinhauser.de, stable@vger.kernel.org, alexander.deucher@amd.com, christian.koenig@amd.com, vbabka@suse.cz List-Id: dri-devel@lists.freedesktop.org VGhpcyBmaXhlcyBhIHJlZ3Jlc3Npb24gaW50cm9kdWNlZCBieSB0aGUgbmV3IGRybV91cGRhdGVf dmJsYW5rX2NvdW50KCkKaW1wbGVtZW50YXRpb24gaW4gTGludXggNC40OgoKUmVzdHJpY3QgdGhl IGJ1bXAgb2YgdGhlIHNvZnR3YXJlIHZibGFuayBjb3VudGVyIGluIGRybV91cGRhdGVfdmJsYW5r X2NvdW50KCkKdG8gYSBzYWZlIG1heGltdW0gdmFsdWUgb2YgKzEgd2hlbmV2ZXIgdGhlcmUgaXMg dGhlIHBvc3NpYmlsaXR5IHRoYXQKY29uY3VycmVudCByZWFkZXJzIG9mIHZibGFuayB0aW1lc3Rh bXBzIGNvdWxkIGJlIGFjdGl2ZSBhdCB0aGUgbW9tZW50LAphcyB0aGUgY3VycmVudCBpbXBsZW1l bnRhdGlvbiBvZiB0aGUgdGltZXN0YW1wIGNhY2hpbmcgYW5kIHVwZGF0aW5nIGlzCm5vdCBzYWZl IGFnYWluc3QgY29uY3VycmVudCByZWFkZXJzIGZvciBjYWxscyB0byBzdG9yZV92YmxhbmsoKSB3 aXRoIGEKYnVtcCBvZiBhbnl0aGluZyBidXQgKzEuIEEgYnVtcCAhPSAxIHdvdWxkIHZlcnkgbGlr ZWx5IHJldHVybiBjb3JydXB0ZWQKdGltZXN0YW1wcyB0byB1c2Vyc3BhY2UsIGJlY2F1c2UgdGhl IHNhbWUgc2xvdCBpbiB0aGUgY2FjaGUgY291bGQKYmUgY29uY3VycmVudGx5IHdyaXR0ZW4gYnkg c3RvcmVfdmJsYW5rKCkgYW5kIHJlYWQgYnkgb25lIG9mIHRob3NlCnJlYWRlcnMgaW4gYSBub24t YXRvbWljIGZhc2hpb24gYW5kIHdpdGhvdXQgdGhlIHJlYWQtcmV0cnkgbG9naWMKZGV0ZWN0aW5n IHRoaXMgY29sbGlzaW9uLgoKQ29uY3VycmVudCByZWFkZXJzIGNhbiBleGlzdCB3aGlsZSBkcm1f dXBkYXRlX3ZibGFua19jb3VudCgpIGlzIGNhbGxlZApmcm9tIHRoZSBkcm1fdmJsYW5rX29mZigp IG9yIGRybV92Ymxhbmtfb24oKSBmdW5jdGlvbnMgb3Igb3RoZXIgbm9uLXZibGFuay0KaXJxIGNh bGxlcnMuIEhvd2V2ZXIsIGFsbCB0aG9zZSBjYWxscyBhcmUgaGFwcGVuaW5nIHdpdGggdGhlIHZi bF9sb2NrCmxvY2tlZCB0aGVyZWJ5IHByZXZlbnRpbmcgYSBkcm1fdmJsYW5rX2dldCgpLCBzbyB0 aGUgdmJsYW5rIHJlZmNvdW50CmNhbid0IGluY3JlYXNlIHdoaWxlIGRybV91cGRhdGVfdmJsYW5r X2NvdW50KCkgaXMgZXhlY3V0aW5nLiBUaGVyZWZvcmUKYSB6ZXJvIHZibGFuayByZWZjb3VudCBk dXJpbmcgZXhlY3V0aW9uIG9mIHRoYXQgZnVuY3Rpb24gc2lnbmFscyB0aGF0CmlzIHNhZmUgZm9y IGFyYml0cmFyeSBjb3VudGVyIGJ1bXBzIGlmIGNhbGxlZCBmcm9tIG91dHNpZGUgdmJsYW5rIGly cSwKd2hlcmVhcyBhIG5vbi16ZXJvIGNvdW50IGlzIG5vdCBzYWZlLgoKV2hlbmV2ZXIgdGhlIGZ1 bmN0aW9uIGlzIGNhbGxlZCBmcm9tIHZibGFuayBpcnEsIHdlIGhhdmUgdG8gYXNzdW1lIGNvbmN1 cnJlbnQKcmVhZGVycyBjb3VsZCBzaG93IHVwIGFueSB0aW1lIGR1cmluZyBpdHMgZXhlY3V0aW9u LCBldmVuIGlmIHRoZSByZWZjb3VudAppcyBjdXJyZW50bHkgemVybywgYXMgdmJsYW5rIGlycXMg YXJlIHVzdWFsbHkgb25seSBlbmFibGVkIGR1ZSB0byB0aGUKcHJlc2VuY2Ugb2YgcmVhZGVycywg YW5kIGJlY2F1c2Ugd2hlbiBpdCBpcyBjYWxsZWQgZnJvbSB2YmxhbmsgaXJxIGl0CmNhbid0IGhv bGQgdGhlIHZibF9sb2NrIHRvIHByb3RlY3QgaXQgZnJvbSBzdWRkZW4gYnVtcHMgaW4gdmJsYW5r IHJlZmNvdW50LgpUaGVyZWZvcmUgYWxzbyByZXN0cmljdCBidW1wcyB0byArMSB3aGVuIHRoZSBm dW5jdGlvbiBpcyBjYWxsZWQgZnJvbSB2YmxhbmsKaXJxLgoKU3VjaCBidW1wcyBvZiBtb3JlIHRo YW4gKzEgY2FuIGhhcHBlbiBhdCBvdGhlciB0aW1lcyB0aGFuIHJlZW5hYmxpbmcKdmJsYW5rIGly cXMsIGUuZy4sIHdoZW4gcmVndWxhciB2YmxhbmsgaW50ZXJydXB0cyBnZXQgZGVsYXllZCBieSBt b3JlCnRoYW4gMSBmcmFtZSBkdWUgdG8gbG9uZyBoZWxkIGxvY2tzLCBsb25nIGlycSBvZmYgcGVy aW9kcywgcmVhbHRpbWUKcHJlZW1wdGlvbiBvbiBSVCBrZXJuZWxzLCBvciBzeXN0ZW0gbWFuYWdl bWVudCBpbnRlcnJ1cHRzLgoKU2lnbmVkLW9mZi1ieTogTWFyaW8gS2xlaW5lciA8bWFyaW8ua2xl aW5lci5kZUBnbWFpbC5jb20+CkNjOiA8c3RhYmxlQHZnZXIua2VybmVsLm9yZz4gIyA0LjQrCkNj OiBtaWNoZWxAZGFlbnplci5uZXQKQ2M6IHZiYWJrYUBzdXNlLmN6CkNjOiB2aWxsZS5zeXJqYWxh QGxpbnV4LmludGVsLmNvbQpDYzogZGFuaWVsLnZldHRlckBmZndsbC5jaApDYzogZHJpLWRldmVs QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpDYzogYWxleGFuZGVyLmRldWNoZXJAYW1kLmNvbQpDYzog Y2hyaXN0aWFuLmtvZW5pZ0BhbWQuY29tCi0tLQogZHJpdmVycy9ncHUvZHJtL2RybV9pcnEuYyB8 IDQxICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrCiAxIGZpbGUgY2hh bmdlZCwgNDEgaW5zZXJ0aW9ucygrKQoKZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9kcm1f aXJxLmMgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2lycS5jCmluZGV4IGJjYjg1MjguLmFhMmM3NGIg MTAwNjQ0Ci0tLSBhL2RyaXZlcnMvZ3B1L2RybS9kcm1faXJxLmMKKysrIGIvZHJpdmVycy9ncHUv ZHJtL2RybV9pcnEuYwpAQCAtMjIxLDYgKzIyMSw0NyBAQCBzdGF0aWMgdm9pZCBkcm1fdXBkYXRl X3ZibGFua19jb3VudChzdHJ1Y3QgZHJtX2RldmljZSAqZGV2LCB1bnNpZ25lZCBpbnQgcGlwZSwK IAkJZGlmZiA9IChmbGFncyAmIERSTV9DQUxMRURfRlJPTV9WQkxJUlEpICE9IDA7CiAJfQogCisJ LyoKKwkgKiBSZXN0cmljdCB0aGUgYnVtcCBvZiB0aGUgc29mdHdhcmUgdmJsYW5rIGNvdW50ZXIg dG8gYSBzYWZlIG1heGltdW0KKwkgKiB2YWx1ZSBvZiArMSB3aGVuZXZlciB0aGVyZSBpcyB0aGUg cG9zc2liaWxpdHkgdGhhdCBjb25jdXJyZW50IHJlYWRlcnMKKwkgKiBvZiB2YmxhbmsgdGltZXN0 YW1wcyBjb3VsZCBiZSBhY3RpdmUgYXQgdGhlIG1vbWVudCwgYXMgdGhlIGN1cnJlbnQKKwkgKiBp bXBsZW1lbnRhdGlvbiBvZiB0aGUgdGltZXN0YW1wIGNhY2hpbmcgYW5kIHVwZGF0aW5nIGlzIG5v dCBzYWZlCisJICogYWdhaW5zdCBjb25jdXJyZW50IHJlYWRlcnMgZm9yIGNhbGxzIHRvIHN0b3Jl X3ZibGFuaygpIHdpdGggYSBidW1wCisJICogb2YgYW55dGhpbmcgYnV0ICsxLiBBIGJ1bXAgIT0g MSB3b3VsZCB2ZXJ5IGxpa2VseSByZXR1cm4gY29ycnVwdGVkCisJICogdGltZXN0YW1wcyB0byB1 c2Vyc3BhY2UsIGJlY2F1c2UgdGhlIHNhbWUgc2xvdCBpbiB0aGUgY2FjaGUgY291bGQKKwkgKiBi ZSBjb25jdXJyZW50bHkgd3JpdHRlbiBieSBzdG9yZV92YmxhbmsoKSBhbmQgcmVhZCBieSBvbmUg b2YgdGhvc2UKKwkgKiByZWFkZXJzIHdpdGhvdXQgdGhlIHJlYWQtcmV0cnkgbG9naWMgZGV0ZWN0 aW5nIHRoZSBjb2xsaXNpb24uCisJICoKKwkgKiBDb25jdXJyZW50IHJlYWRlcnMgY2FuIGV4aXN0 IHdoZW4gd2UgYXJlIGNhbGxlZCBmcm9tIHRoZQorCSAqIGRybV92Ymxhbmtfb2ZmKCkgb3IgZHJt X3ZibGFua19vbigpIGZ1bmN0aW9ucyBhbmQgb3RoZXIgbm9uLXZibGFuay0KKwkgKiBpcnEgY2Fs bGVycy4gSG93ZXZlciwgYWxsIHRob3NlIGNhbGxzIHRvIHVzIGFyZSBoYXBwZW5pbmcgd2l0aCB0 aGUKKwkgKiB2YmxfbG9jayBsb2NrZWQgdG8gcHJldmVudCBkcm1fdmJsYW5rX2dldCgpLCBzbyB0 aGUgdmJsYW5rIHJlZmNvdW50CisJICogY2FuJ3QgaW5jcmVhc2Ugd2hpbGUgd2UgYXJlIGV4ZWN1 dGluZy4gVGhlcmVmb3JlIGEgemVybyByZWZjb3VudCBhdAorCSAqIHRoaXMgcG9pbnQgaXMgc2Fm ZSBmb3IgYXJiaXRyYXJ5IGNvdW50ZXIgYnVtcHMgaWYgd2UgYXJlIGNhbGxlZAorCSAqIG91dHNp ZGUgdmJsYW5rIGlycSwgYSBub24temVybyBjb3VudCBpcyBub3QgMTAwJSBzYWZlLiBVbmZvcnR1 bmF0ZWx5CisJICogd2UgbXVzdCBhbHNvIGFjY2VwdCBhIHJlZmNvdW50IG9mIDEsIGFzIHdoZW5l dmVyIHdlIGFyZSBjYWxsZWQgZnJvbQorCSAqIGRybV92YmxhbmtfZ2V0KCkgLT4gZHJtX3ZibGFu a19lbmFibGUoKSB0aGUgcmVmY291bnQgd2lsbCBiZSAxIGFuZAorCSAqIHdlIG11c3QgbGV0IHRo YXQgb25lIHBhc3MgdGhyb3VnaCBpbiBvcmRlciB0byBub3QgbG9zZSB2YmxhbmsgY291bnRzCisJ ICogZHVyaW5nIHZibGFuayBpcnEgb2ZmIC0gd2hpY2ggd291bGQgY29tcGxldGVseSBkZWZlYXQg dGhlIHdob2xlCisJICogcG9pbnQgb2YgdGhpcyByb3V0aW5lLgorCSAqCisJICogV2hlbmV2ZXIg d2UgYXJlIGNhbGxlZCBmcm9tIHZibGFuayBpcnEsIHdlIGhhdmUgdG8gYXNzdW1lIGNvbmN1cnJl bnQKKwkgKiByZWFkZXJzIGV4aXN0IG9yIGNhbiBzaG93IHVwIGFueSB0aW1lIGR1cmluZyBvdXIg ZXhlY3V0aW9uLCBldmVuIGlmCisJICogdGhlIHJlZmNvdW50IGlzIGN1cnJlbnRseSB6ZXJvLCBh cyB2YmxhbmsgaXJxcyBhcmUgdXN1YWxseSBvbmx5CisJICogZW5hYmxlZCBkdWUgdG8gdGhlIHBy ZXNlbmNlIG9mIHJlYWRlcnMsIGFuZCBiZWNhdXNlIHdoZW4gd2UgYXJlIGNhbGxlZAorCSAqIGZy b20gdmJsYW5rIGlycSB3ZSBjYW4ndCBob2xkIHRoZSB2YmxfbG9jayB0byBwcm90ZWN0IHVzIGZy b20gc3VkZGVuCisJICogYnVtcHMgaW4gdmJsYW5rIHJlZmNvdW50LiBUaGVyZWZvcmUgYWxzbyBy ZXN0cmljdCBidW1wcyB0byArMSB3aGVuCisJICogY2FsbGVkIGZyb20gdmJsYW5rIGlycS4KKwkg Ki8KKwlpZiAoKGRpZmYgPiAxKSAmJiAoYXRvbWljX3JlYWQoJnZibGFuay0+cmVmY291bnQpID4g MSB8fAorCSAgICAoZmxhZ3MgJiBEUk1fQ0FMTEVEX0ZST01fVkJMSVJRKSkpIHsKKwkJRFJNX0RF QlVHX1ZCTCgiY2xhbXBpbmcgdmJsYW5rIGJ1bXAgdG8gMSBvbiBjcnRjICV1OiBkaWZmcj0ldSAi CisJCQkgICAgICAicmVmY291bnQgJXUsIHZibGlycSAldVxuIiwgcGlwZSwgZGlmZiwKKwkJCSAg ICAgIGF0b21pY19yZWFkKCZ2YmxhbmstPnJlZmNvdW50KSwKKwkJCSAgICAgIChmbGFncyAmIERS TV9DQUxMRURfRlJPTV9WQkxJUlEpICE9IDApOworCQlkaWZmID0gMTsKKwl9CisKIAlEUk1fREVC VUdfVkJMKCJ1cGRhdGluZyB2YmxhbmsgY291bnQgb24gY3J0YyAldToiCiAJCSAgICAgICIgY3Vy cmVudD0ldSwgZGlmZj0ldSwgaHc9JXUgaHdfbGFzdD0ldVxuIiwKIAkJICAgICAgcGlwZSwgdmJs YW5rLT5jb3VudCwgZGlmZiwgY3VyX3ZibGFuaywgdmJsYW5rLT5sYXN0KTsKLS0gCjEuOS4xCgpf X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwg bWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK