From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1D36CC433EF for ; Tue, 15 Mar 2022 04:54:19 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CCADB89D4B; Tue, 15 Mar 2022 04:54:17 +0000 (UTC) Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by gabe.freedesktop.org (Postfix) with ESMTPS id 519628961E; Tue, 15 Mar 2022 04:54:16 +0000 (UTC) Received: from [192.168.1.111] (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 11C60929; Tue, 15 Mar 2022 05:54:13 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1647320054; bh=khtrebC3HtM23l2NbquJCCu0hR6BMa5dWzTk/g6cm+U=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=i3TCDq55p7VCRQmWmoPujgKC92l+uqY5KW2R9KfLMHjhetT/ws04LdQmYxXJj12/2 7drcMYrj8LD5fKjE9cFGiWuhF2b6GWgklAfGD/7ZAO+MtB6SAWoSG9SzyA5UbUlkYN CgbTv8sLWNTKMSzReDibc3eUu7NKiYOGr9bCN/r4= Message-ID: <68e180f9-d646-7aed-0f2c-b6caeb8877fe@ideasonboard.com> Date: Tue, 15 Mar 2022 06:54:11 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH 16/22] drm/tilcdc: Use drm_mode_copy() Content-Language: en-US To: Ville Syrjala , dri-devel@lists.freedesktop.org References: <20220218100403.7028-1-ville.syrjala@linux.intel.com> <20220218100403.7028-17-ville.syrjala@linux.intel.com> From: Tomi Valkeinen In-Reply-To: <20220218100403.7028-17-ville.syrjala@linux.intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org, Jyri Sarha Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 18/02/2022 12:03, Ville Syrjala wrote: > From: Ville Syrjälä > > struct drm_display_mode embeds a list head, so overwriting > the full struct with another one will corrupt the list > (if the destination mode is on a list). Use drm_mode_copy() > instead which explicitly preserves the list head of > the destination mode. > > Even if we know the destination mode is not on any list > using drm_mode_copy() seems decent as it sets a good > example. Bad examples of not using it might eventually > get copied into code where preserving the list head > actually matters. > > Obviously one case not covered here is when the mode > itself is embedded in a larger structure and the whole > structure is copied. But if we are careful when copying > into modes embedded in structures I think we can be a > little more reassured that bogus list heads haven't been > propagated in. > > @is_mode_copy@ > @@ > drm_mode_copy(...) > { > ... > } > > @depends on !is_mode_copy@ > struct drm_display_mode *mode; > expression E, S; > @@ > ( > - *mode = E > + drm_mode_copy(mode, &E) > | > - memcpy(mode, E, S) > + drm_mode_copy(mode, E) > ) > > @depends on !is_mode_copy@ > struct drm_display_mode mode; > expression E; > @@ > ( > - mode = E > + drm_mode_copy(&mode, &E) > | > - memcpy(&mode, E, S) > + drm_mode_copy(&mode, E) > ) > > @@ > struct drm_display_mode *mode; > @@ > - &*mode > + mode > > Cc: Jyri Sarha > Cc: Tomi Valkeinen > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > index 29890d704cb4..853c6b443fff 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > @@ -433,7 +433,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc) > > set_scanout(crtc, fb); > > - crtc->hwmode = crtc->state->adjusted_mode; > + drm_mode_copy(&crtc->hwmode, &crtc->state->adjusted_mode); > > tilcdc_crtc->hvtotal_us = > tilcdc_mode_hvtotal(&crtc->hwmode); Reviewed-by: Tomi Valkeinen Tomi From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D237CC433F5 for ; Tue, 15 Mar 2022 04:54:23 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7578189F2A; Tue, 15 Mar 2022 04:54:18 +0000 (UTC) Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by gabe.freedesktop.org (Postfix) with ESMTPS id 519628961E; Tue, 15 Mar 2022 04:54:16 +0000 (UTC) Received: from [192.168.1.111] (91-156-85-209.elisa-laajakaista.fi [91.156.85.209]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 11C60929; Tue, 15 Mar 2022 05:54:13 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1647320054; bh=khtrebC3HtM23l2NbquJCCu0hR6BMa5dWzTk/g6cm+U=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=i3TCDq55p7VCRQmWmoPujgKC92l+uqY5KW2R9KfLMHjhetT/ws04LdQmYxXJj12/2 7drcMYrj8LD5fKjE9cFGiWuhF2b6GWgklAfGD/7ZAO+MtB6SAWoSG9SzyA5UbUlkYN CgbTv8sLWNTKMSzReDibc3eUu7NKiYOGr9bCN/r4= Message-ID: <68e180f9-d646-7aed-0f2c-b6caeb8877fe@ideasonboard.com> Date: Tue, 15 Mar 2022 06:54:11 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Content-Language: en-US To: Ville Syrjala , dri-devel@lists.freedesktop.org References: <20220218100403.7028-1-ville.syrjala@linux.intel.com> <20220218100403.7028-17-ville.syrjala@linux.intel.com> From: Tomi Valkeinen In-Reply-To: <20220218100403.7028-17-ville.syrjala@linux.intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Intel-gfx] [PATCH 16/22] drm/tilcdc: Use drm_mode_copy() X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org, Jyri Sarha Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On 18/02/2022 12:03, Ville Syrjala wrote: > From: Ville Syrjälä > > struct drm_display_mode embeds a list head, so overwriting > the full struct with another one will corrupt the list > (if the destination mode is on a list). Use drm_mode_copy() > instead which explicitly preserves the list head of > the destination mode. > > Even if we know the destination mode is not on any list > using drm_mode_copy() seems decent as it sets a good > example. Bad examples of not using it might eventually > get copied into code where preserving the list head > actually matters. > > Obviously one case not covered here is when the mode > itself is embedded in a larger structure and the whole > structure is copied. But if we are careful when copying > into modes embedded in structures I think we can be a > little more reassured that bogus list heads haven't been > propagated in. > > @is_mode_copy@ > @@ > drm_mode_copy(...) > { > ... > } > > @depends on !is_mode_copy@ > struct drm_display_mode *mode; > expression E, S; > @@ > ( > - *mode = E > + drm_mode_copy(mode, &E) > | > - memcpy(mode, E, S) > + drm_mode_copy(mode, E) > ) > > @depends on !is_mode_copy@ > struct drm_display_mode mode; > expression E; > @@ > ( > - mode = E > + drm_mode_copy(&mode, &E) > | > - memcpy(&mode, E, S) > + drm_mode_copy(&mode, E) > ) > > @@ > struct drm_display_mode *mode; > @@ > - &*mode > + mode > > Cc: Jyri Sarha > Cc: Tomi Valkeinen > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > index 29890d704cb4..853c6b443fff 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > @@ -433,7 +433,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc) > > set_scanout(crtc, fb); > > - crtc->hwmode = crtc->state->adjusted_mode; > + drm_mode_copy(&crtc->hwmode, &crtc->state->adjusted_mode); > > tilcdc_crtc->hvtotal_us = > tilcdc_mode_hvtotal(&crtc->hwmode); Reviewed-by: Tomi Valkeinen Tomi