All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 01/22] drm: Add drm_mode_init()
Date: Fri, 18 Feb 2022 13:56:58 +0200	[thread overview]
Message-ID: <Yg+JivyhsNufopPc@intel.com> (raw)
In-Reply-To: <0e2baaab-6a38-5f80-5552-57326d781c43@intel.com>

On Fri, Feb 18, 2022 at 12:22:44PM +0100, Andrzej Hajda wrote:
> 
> 
> On 18.02.2022 11:03, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Add a variant of drm_mode_copy() that explicitly clears out
> > the list head of the destination mode. Helpful to guarantee
> > we don't have stack garbage left in there for on-stack modes.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/drm_modes.c | 17 +++++++++++++++++
> >   include/drm/drm_modes.h     |  2 ++
> >   2 files changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index 96b13e36293c..40d4ce4a1da4 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -892,6 +892,23 @@ void drm_mode_copy(struct drm_display_mode *dst, const struct drm_display_mode *
> >   }
> >   EXPORT_SYMBOL(drm_mode_copy);
> >   
> > +/**
> > + * drm_mode_init - initialize the mode from another mode
> > + * @dst: mode to overwrite
> > + * @src: mode to copy
> > + *
> > + * Copy an existing mode into another mode, zeroing the
> > + * list head of the destination mode. Typically used
> > + * to guarantee the list head is not left with stack
> > + * garbage in on-stack modes.
> > + */
> > +void drm_mode_init(struct drm_display_mode *dst, const struct drm_display_mode *src)
> > +{
> > +	memset(dst, 0, sizeof(*dst));
> 
> Why not just clear the list head? Or maybe poison it? It would be more 
> cleaner.

Then we have two places that need to be updated if some other field
gets introduced that needs preserving. With a full memset() we only
have to care about drm_mode_copy(). Don't see much point in
micro-optimizing this thing.

-- 
Ville Syrjälä
Intel

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 01/22] drm: Add drm_mode_init()
Date: Fri, 18 Feb 2022 13:56:58 +0200	[thread overview]
Message-ID: <Yg+JivyhsNufopPc@intel.com> (raw)
In-Reply-To: <0e2baaab-6a38-5f80-5552-57326d781c43@intel.com>

On Fri, Feb 18, 2022 at 12:22:44PM +0100, Andrzej Hajda wrote:
> 
> 
> On 18.02.2022 11:03, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Add a variant of drm_mode_copy() that explicitly clears out
> > the list head of the destination mode. Helpful to guarantee
> > we don't have stack garbage left in there for on-stack modes.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/drm_modes.c | 17 +++++++++++++++++
> >   include/drm/drm_modes.h     |  2 ++
> >   2 files changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index 96b13e36293c..40d4ce4a1da4 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -892,6 +892,23 @@ void drm_mode_copy(struct drm_display_mode *dst, const struct drm_display_mode *
> >   }
> >   EXPORT_SYMBOL(drm_mode_copy);
> >   
> > +/**
> > + * drm_mode_init - initialize the mode from another mode
> > + * @dst: mode to overwrite
> > + * @src: mode to copy
> > + *
> > + * Copy an existing mode into another mode, zeroing the
> > + * list head of the destination mode. Typically used
> > + * to guarantee the list head is not left with stack
> > + * garbage in on-stack modes.
> > + */
> > +void drm_mode_init(struct drm_display_mode *dst, const struct drm_display_mode *src)
> > +{
> > +	memset(dst, 0, sizeof(*dst));
> 
> Why not just clear the list head? Or maybe poison it? It would be more 
> cleaner.

Then we have two places that need to be updated if some other field
gets introduced that needs preserving. With a full memset() we only
have to care about drm_mode_copy(). Don't see much point in
micro-optimizing this thing.

-- 
Ville Syrjälä
Intel

  reply	other threads:[~2022-02-18 11:57 UTC|newest]

Thread overview: 157+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-18 10:03 [PATCH 00/22] drm: Review of mode copies Ville Syrjala
2022-02-18 10:03 ` Ville Syrjala
2022-02-18 10:03 ` Ville Syrjala
2022-02-18 10:03 ` Ville Syrjala
2022-02-18 10:03 ` Ville Syrjala
2022-02-18 10:03 ` [Intel-gfx] " Ville Syrjala
2022-02-18 10:03 ` [Intel-gfx] [PATCH 01/22] drm: Add drm_mode_init() Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 11:22   ` [Intel-gfx] " Andrzej Hajda
2022-02-18 11:22     ` Andrzej Hajda
2022-02-18 11:56     ` Ville Syrjälä [this message]
2022-02-18 11:56       ` Ville Syrjälä
2022-02-18 12:23       ` [Intel-gfx] " Andrzej Hajda
2022-02-18 12:23         ` Andrzej Hajda
2022-02-18 16:34   ` [Intel-gfx] " Harry Wentland
2022-02-18 16:34     ` Harry Wentland
2022-02-18 10:03 ` [PATCH 02/22] drm/amdgpu: Remove pointless on stack mode copies Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03   ` [Intel-gfx] " Ville Syrjala
2022-02-18 16:28   ` Harry Wentland
2022-02-18 16:28     ` [Intel-gfx] " Harry Wentland
2022-03-15 15:57     ` Alex Deucher
2022-03-15 15:57       ` Alex Deucher
2022-03-15 15:57       ` [Intel-gfx] " Alex Deucher
2022-02-18 10:03 ` [PATCH 03/22] drm/amdgpu: Use drm_mode_init() for on-stack modes Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03   ` [Intel-gfx] " Ville Syrjala
2022-02-18 16:30   ` Harry Wentland
2022-02-18 16:30     ` [Intel-gfx] " Harry Wentland
2022-02-18 10:03 ` [PATCH 04/22] drm/amdgpu: Use drm_mode_copy() Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03   ` [Intel-gfx] " Ville Syrjala
2022-02-18 16:32   ` Harry Wentland
2022-02-18 16:32     ` [Intel-gfx] " Harry Wentland
2022-03-15 18:50     ` Alex Deucher
2022-03-15 18:50       ` Alex Deucher
2022-03-15 18:50       ` [Intel-gfx] " Alex Deucher
2022-02-18 10:03 ` [PATCH 05/22] drm/radeon: " Ville Syrjala
2022-02-18 10:03   ` [Intel-gfx] " Ville Syrjala
2022-03-15 18:49   ` Alex Deucher
2022-03-15 18:49     ` [Intel-gfx] " Alex Deucher
2022-02-18 10:03 ` [Intel-gfx] [PATCH 06/22] drm/bridge: " Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 11:25   ` [Intel-gfx] " Andrzej Hajda
2022-02-18 11:25     ` Andrzej Hajda
2022-02-18 17:47   ` [Intel-gfx] " Laurent Pinchart
2022-02-18 17:47     ` Laurent Pinchart
2022-02-18 10:03 ` [Intel-gfx] [PATCH 07/22] drm/gma500: " Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-03-16 13:16   ` [Intel-gfx] " Patrik Jakobsson
2022-03-16 13:16     ` Patrik Jakobsson
2022-02-18 10:03 ` [Intel-gfx] [PATCH 08/22] drm/hisilicon: Use drm_mode_init() for on-stack modes Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03 ` [Intel-gfx] [PATCH 09/22] drm/imx: Use drm_mode_duplicate() Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 11:42   ` [Intel-gfx] " Philipp Zabel
2022-02-18 11:42     ` Philipp Zabel
2022-02-18 10:03 ` [Intel-gfx] [PATCH 10/22] drm/msm: Nuke weird on stack mode copy Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-03-23 10:19   ` [Intel-gfx] " Dmitry Baryshkov
2022-03-23 10:19     ` Dmitry Baryshkov
2022-03-23 10:19     ` Dmitry Baryshkov
2022-02-18 10:03 ` [Intel-gfx] [PATCH 11/22] drm/msm: Use drm_mode_init() for on-stack modes Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-03-23 10:11   ` [Intel-gfx] " Dmitry Baryshkov
2022-03-23 10:11     ` Dmitry Baryshkov
2022-03-23 10:11     ` Dmitry Baryshkov
2022-03-23 20:04   ` [Intel-gfx] " Abhinav Kumar
2022-03-23 20:04     ` Abhinav Kumar
2022-03-23 20:04     ` Abhinav Kumar
2022-02-18 10:03 ` [Intel-gfx] [PATCH 12/22] drm/msm: Use drm_mode_copy() Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-03-23 10:12   ` [Intel-gfx] " Dmitry Baryshkov
2022-03-23 10:12     ` Dmitry Baryshkov
2022-03-23 10:12     ` Dmitry Baryshkov
2022-03-23 20:09   ` [Intel-gfx] " Abhinav Kumar
2022-03-23 20:09     ` Abhinav Kumar
2022-03-23 20:09     ` Abhinav Kumar
2022-02-18 10:03 ` [Intel-gfx] [PATCH 13/22] drm/mtk: Use drm_mode_init() for on-stack modes Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03 ` [Intel-gfx] [PATCH 14/22] drm/rockchip: Use drm_mode_copy() Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03 ` [Intel-gfx] [PATCH 15/22] drm/sti: " Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-18 10:03 ` [Intel-gfx] [PATCH 16/22] drm/tilcdc: " Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-03-15  4:54   ` [Intel-gfx] " Tomi Valkeinen
2022-03-15  4:54     ` Tomi Valkeinen
2022-02-18 10:03 ` [Intel-gfx] [PATCH 17/22] drm/vc4: " Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-02-21 10:13   ` [Intel-gfx] (subset) " Maxime Ripard
2022-02-21 10:13     ` (subset) [Intel-gfx] " Maxime Ripard
2022-02-18 10:03 ` [Intel-gfx] [PATCH 18/22] drm/i915: Use drm_mode_init() for on-stack modes Ville Syrjala
2022-02-18 10:03   ` Ville Syrjala
2022-03-16  8:00   ` [Intel-gfx] " Jani Nikula
2022-03-21 18:57     ` [cocci] " Ville Syrjälä
2022-03-21 18:57       ` Ville Syrjälä
2022-03-21 20:48       ` [cocci] " Julia Lawall
2022-03-21 20:48         ` Julia Lawall
2022-02-18 10:04 ` [Intel-gfx] [PATCH 19/22] drm/i915: Use drm_mode_copy() Ville Syrjala
2022-02-18 10:04   ` Ville Syrjala
2022-03-16  8:04   ` [Intel-gfx] " Jani Nikula
2022-02-18 10:04 ` [Intel-gfx] [PATCH 20/22] drm/panel: Use drm_mode_duplicate() Ville Syrjala
2022-02-18 10:04   ` Ville Syrjala
2022-02-18 11:51   ` [Intel-gfx] " Sam Ravnborg
2022-02-18 11:51     ` Sam Ravnborg
2022-02-18 10:04 ` [Intel-gfx] [PATCH 21/22] drm: Use drm_mode_init() for on-stack modes Ville Syrjala
2022-02-18 10:04   ` Ville Syrjala
2022-03-22 11:10   ` [Intel-gfx] " Andrzej Hajda
2022-03-22 11:10     ` Andrzej Hajda
2022-02-18 10:04 ` [Intel-gfx] [PATCH 22/22] drm: Use drm_mode_copy() Ville Syrjala
2022-02-18 10:04   ` Ville Syrjala
2022-03-22 11:19   ` [Intel-gfx] " Andrzej Hajda
2022-03-22 11:19     ` Andrzej Hajda
2022-02-18 13:43 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm: Review of mode copies Patchwork
2022-02-18 14:16 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-02-19  5:06 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2022-03-14 22:11 ` [PATCH 00/22] " Ville Syrjälä
2022-03-14 22:11   ` Ville Syrjälä
2022-03-14 22:11   ` Ville Syrjälä
2022-03-14 22:11   ` Ville Syrjälä
2022-03-14 22:11   ` Ville Syrjälä
2022-03-14 22:11   ` [Intel-gfx] " Ville Syrjälä
2022-03-15 18:52   ` Alex Deucher
2022-03-15 18:52     ` Alex Deucher
2022-03-15 18:52     ` Alex Deucher
2022-03-15 18:52     ` Alex Deucher
2022-03-15 18:52     ` Alex Deucher
2022-03-15 18:52     ` [Intel-gfx] " Alex Deucher
2022-03-21 22:37     ` Ville Syrjälä
2022-03-21 22:37       ` Ville Syrjälä
2022-03-21 22:37       ` Ville Syrjälä
2022-03-21 22:37       ` Ville Syrjälä
2022-03-21 22:37       ` Ville Syrjälä
2022-03-21 22:37       ` [Intel-gfx] " Ville Syrjälä
2022-03-23 10:39       ` Dmitry Baryshkov
2022-03-23 10:39         ` Dmitry Baryshkov
2022-03-23 10:39         ` Dmitry Baryshkov
2022-03-23 10:39         ` Dmitry Baryshkov
2022-03-23 10:39         ` Dmitry Baryshkov
2022-03-23 10:39         ` [Intel-gfx] " Dmitry Baryshkov
2022-03-23 15:10         ` Ville Syrjälä
2022-03-23 15:10           ` Ville Syrjälä
2022-03-23 15:10           ` Ville Syrjälä
2022-03-23 15:10           ` Ville Syrjälä
2022-03-23 15:10           ` Ville Syrjälä
2022-03-23 15:10           ` [Intel-gfx] " Ville Syrjälä
2022-03-23 20:50         ` Abhinav Kumar
2022-03-23 20:50           ` Abhinav Kumar
2022-03-23 20:50           ` Abhinav Kumar
2022-03-23 20:50           ` Abhinav Kumar
2022-03-23 20:50           ` [Intel-gfx] " Abhinav Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yg+JivyhsNufopPc@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=andrzej.hajda@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.