From patchwork Thu Oct 4 22:38:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Hellstrom X-Patchwork-Id: 10626959 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 21BD1112B for ; Thu, 4 Oct 2018 22:38:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 12A00290AC for ; Thu, 4 Oct 2018 22:38:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 06465290AF; Thu, 4 Oct 2018 22:38:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 80DC3290AC for ; Thu, 4 Oct 2018 22:38:28 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 171496E67C; Thu, 4 Oct 2018 22:38:27 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from NAM04-CO1-obe.outbound.protection.outlook.com (mail-eopbgr690054.outbound.protection.outlook.com [40.107.69.54]) by gabe.freedesktop.org (Postfix) with ESMTPS id AE0436E67C for ; Thu, 4 Oct 2018 22:38:25 +0000 (UTC) Received: from SN6PR05MB4589.namprd05.prod.outlook.com (52.135.75.151) by SN6PR05MB4672.namprd05.prod.outlook.com (52.135.114.206) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1228.12; Thu, 4 Oct 2018 22:38:23 +0000 Received: from SN6PR05MB4589.namprd05.prod.outlook.com ([fe80::38a5:a632:bd0f:c117]) by SN6PR05MB4589.namprd05.prod.outlook.com ([fe80::38a5:a632:bd0f:c117%4]) with mapi id 15.20.1207.021; Thu, 4 Oct 2018 22:38:23 +0000 From: Thomas Hellstrom To: "dri-devel@lists.freedesktop.org" Subject: [PATCH 2/2] drm/vmwgfx: Fix a layout race-condition Thread-Topic: [PATCH 2/2] drm/vmwgfx: Fix a layout race-condition Thread-Index: AQHUXDL1e7/5CuYQv0OdBy2QrNbuuA== Date: Thu, 4 Oct 2018 22:38:23 +0000 Message-ID: <20181004223753.22846-2-thellstrom@vmware.com> References: <20181004223753.22846-1-thellstrom@vmware.com> In-Reply-To: <20181004223753.22846-1-thellstrom@vmware.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: DM5PR06CA0103.namprd06.prod.outlook.com (2603:10b6:4:3a::44) To SN6PR05MB4589.namprd05.prod.outlook.com (2603:10b6:805:38::23) x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [66.170.99.1] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; SN6PR05MB4672; 20:W9eOgnzfP9GOcrDIvlCw2s3IHot2++ZShsujOwbl+MskpgjPtxLF5X7Ld9dIadPo/EB27278ymxE/hSzZYbeZzdzt57Schl41dk1mgNII+IXcslkL+PeuvRFbAN0SPnmvu45GJhIeyKwoBPnSnXQAzKlkd0Epm0nWZI4fKmAnZY= x-ms-office365-filtering-correlation-id: ff915b3d-2db8-43fe-5634-08d62a4a179c x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(2017052603328)(7153060)(7193020); SRVR:SN6PR05MB4672; x-ms-traffictypediagnostic: SN6PR05MB4672: x-ld-processed: b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0,ExtAddr bcl: 0 x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(61668805478150); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(5005006)(8121501046)(93006095)(93001095)(10201501046)(3231355)(944501410)(52105095)(3002001)(149066)(150057)(6041310)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(20161123562045)(20161123558120)(20161123560045)(201708071742011)(7699051); SRVR:SN6PR05MB4672; BCL:0; PCL:0; RULEID:; SRVR:SN6PR05MB4672; x-forefront-prvs: 0815F8251E x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39860400002)(346002)(376002)(136003)(366004)(396003)(199004)(189003)(86362001)(575784001)(2616005)(102836004)(68736007)(99286004)(6486002)(71190400001)(71200400001)(476003)(26005)(105586002)(76176011)(52116002)(2351001)(106356001)(6506007)(386003)(5660300001)(6916009)(107886003)(25786009)(4326008)(186003)(6512007)(14454004)(2900100001)(2906002)(8676002)(6436002)(81156014)(81166006)(5250100002)(316002)(36756003)(305945005)(1076002)(446003)(5640700003)(11346002)(14444005)(256004)(2501003)(66066001)(486006)(8936002)(7736002)(478600001)(54906003)(53936002)(3846002)(6116002)(97736004); DIR:OUT; SFP:1101; SCL:1; SRVR:SN6PR05MB4672; H:SN6PR05MB4589.namprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: vmware.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: iExqUyhfMBzKzP8ptmiCg1FnbRFfw61Fshc3quC6Y/9/GwpMjX6OJkAEScbyJoGangiKf7H8+s07hsI8IQX0mXcCIlw5Vz/8eTzsiwFRVeU+UD5wTKSF1alisGZlPaoV1kCBi5XvKSTOWkVs1yDD92AIuY8Er+QAEQL/sMK5ZInis0Ym0OQ8+ccYaihbm7QbPLV48FapBDsR4PbpkbHCuZNPyKoe6p4cVq+/0aJ3PJjRXYs27rruy/TceIJYbJIe/2J8B5c+pw6mrcJvhnQl35hCpA8oj54Da+AE25ZjNX2EIkSYAiIxnmHxoi8D+ZqjCVOrhebjRFXn2bk/c3T7NrGoOqq2ZNrmP+y4B6Jlz2w= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: vmware.com X-MS-Exchange-CrossTenant-Network-Message-Id: ff915b3d-2db8-43fe-5634-08d62a4a179c X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Oct 2018 22:38:23.5164 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR05MB4672 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "daniel.vetter@ffwll.ch" , Thomas Hellstrom , linux-graphics-maintainer Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP This fixes a layout update race condition. We make sure the crtc mutex is locked before we dereference crtc->state. Otherwise the state might change under us. Since now we're already holding the crtc mutexes when reading the gui coordinates, protect them with the crtc mutexes rather than with the requested_layout mutex. Signed-off-by: Thomas Hellstrom Reviewed-by: Deepak Rawat Reviewed-by: Sinclair Yeh --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 - drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 9 ------ drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 64 ++++++++++++++++++++++--------------- 3 files changed, 38 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 61a84b958d67..8107753869e0 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -665,7 +665,6 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) mutex_init(&dev_priv->cmdbuf_mutex); mutex_init(&dev_priv->release_mutex); mutex_init(&dev_priv->binding_mutex); - mutex_init(&dev_priv->requested_layout_mutex); mutex_init(&dev_priv->global_kms_state_mutex); ttm_lock_init(&dev_priv->reservation_sem); spin_lock_init(&dev_priv->resource_lock); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h index ea2c22d92357..aad9942a3f54 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h @@ -467,15 +467,6 @@ struct vmw_private { uint32_t num_displays; - /* - * Currently requested_layout_mutex is used to protect the gui - * positionig state in display unit. With that use case currently this - * mutex is only taken during layout ioctl and atomic check_modeset. - * Other display unit state can be protected with this mutex but that - * needs careful consideration. - */ - struct mutex requested_layout_mutex; - /* * Framebuffer info. */ diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index ccaec2cbabd2..1e0f3ff61d21 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -1646,7 +1646,6 @@ static int vmw_kms_check_implicit(struct drm_device *dev, static int vmw_kms_check_topology(struct drm_device *dev, struct drm_atomic_state *state) { - struct vmw_private *dev_priv = vmw_priv(dev); struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_rect *rects; struct drm_crtc *crtc; @@ -1658,19 +1657,31 @@ static int vmw_kms_check_topology(struct drm_device *dev, if (!rects) return -ENOMEM; - mutex_lock(&dev_priv->requested_layout_mutex); - drm_for_each_crtc(crtc, dev) { struct vmw_display_unit *du = vmw_crtc_to_du(crtc); - struct drm_crtc_state *crtc_state = crtc->state; + struct drm_crtc_state *crtc_state; i = drm_crtc_index(crtc); - if (crtc_state && crtc_state->enable) { + crtc_state = vmw_crtc_state_and_lock(state, crtc); + if (IS_ERR(crtc_state)) { + ret = PTR_ERR(crtc_state); + goto clean; + } + + if (!crtc_state) + continue; + + if (crtc_state->enable) { rects[i].x1 = du->gui_x; rects[i].y1 = du->gui_y; rects[i].x2 = du->gui_x + crtc_state->mode.hdisplay; rects[i].y2 = du->gui_y + crtc_state->mode.vdisplay; + } else { + rects[i].x1 = 0; + rects[i].y1 = 0; + rects[i].x2 = 0; + rects[i].y2 = 0; } } @@ -1682,14 +1693,6 @@ static int vmw_kms_check_topology(struct drm_device *dev, struct drm_connector_state *conn_state; struct vmw_connector_state *vmw_conn_state; - if (!new_crtc_state->enable && old_crtc_state->enable) { - rects[i].x1 = 0; - rects[i].y1 = 0; - rects[i].x2 = 0; - rects[i].y2 = 0; - continue; - } - if (!du->pref_active) { ret = -EINVAL; goto clean; @@ -1710,18 +1713,12 @@ static int vmw_kms_check_topology(struct drm_device *dev, vmw_conn_state = vmw_connector_state_to_vcs(conn_state); vmw_conn_state->gui_x = du->gui_x; vmw_conn_state->gui_y = du->gui_y; - - rects[i].x1 = du->gui_x; - rects[i].y1 = du->gui_y; - rects[i].x2 = du->gui_x + new_crtc_state->mode.hdisplay; - rects[i].y2 = du->gui_y + new_crtc_state->mode.vdisplay; } ret = vmw_kms_check_display_memory(dev, dev->mode_config.num_crtc, rects); clean: - mutex_unlock(&dev_priv->requested_layout_mutex); kfree(rects); return ret; } @@ -2078,11 +2075,25 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, struct vmw_display_unit *du; struct drm_connector *con; struct drm_connector_list_iter conn_iter; + struct drm_modeset_acquire_ctx ctx; + struct drm_crtc *crtc; + int ret; + + /* Currently gui_x/y is protected with the crtc mutex */ + mutex_lock(&dev->mode_config.mutex); + drm_modeset_acquire_init(&ctx, 0); +retry: + drm_for_each_crtc(crtc, dev) { + ret = drm_modeset_lock(&crtc->mutex, &ctx); + if (ret < 0) { + if (ret == -EDEADLK) { + drm_modeset_backoff(&ctx); + goto retry; + } + goto out_fini; + } + } - /* - * Currently only gui_x/y is protected with requested_layout_mutex. - */ - mutex_lock(&dev_priv->requested_layout_mutex); drm_connector_list_iter_begin(dev, &conn_iter); drm_for_each_connector_iter(con, &conn_iter) { du = vmw_connector_to_du(con); @@ -2101,9 +2112,7 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, } } drm_connector_list_iter_end(&conn_iter); - mutex_unlock(&dev_priv->requested_layout_mutex); - mutex_lock(&dev->mode_config.mutex); list_for_each_entry(con, &dev->mode_config.connector_list, head) { du = vmw_connector_to_du(con); if (num_rects > du->unit) { @@ -2123,9 +2132,12 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, } con->status = vmw_du_connector_detect(con, true); } - mutex_unlock(&dev->mode_config.mutex); drm_sysfs_hotplug_event(dev); +out_fini: + drm_modeset_drop_locks(&ctx); + drm_modeset_acquire_fini(&ctx); + mutex_unlock(&dev->mode_config.mutex); return 0; }