* [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
@ 2018-01-24 18:37 Ville Syrjala
2018-01-24 19:21 ` ✗ Fi.CI.BAT: warning for " Patchwork
` (7 more replies)
0 siblings, 8 replies; 18+ messages in thread
From: Ville Syrjala @ 2018-01-24 18:37 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
Naturally we can only do that if the index of those objects stays
below 32. Issue a warning whenever we exceed that limit, hopefully
prompting someone to fix the problem.
Or should we just outright fail the object initializatio perhaps?
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_connector.c | 3 +++
drivers/gpu/drm/drm_crtc.c | 3 +++
drivers/gpu/drm/drm_encoder.c | 3 +++
drivers/gpu/drm/drm_plane.c | 3 +++
4 files changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b85a7749709d..9278a81c5d54 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -211,6 +211,9 @@ int drm_connector_init(struct drm_device *dev,
connector->index = ret;
ret = 0;
+ /* we have 32bit connector bitmasks */
+ WARN_ON(connector->index >= 32);
+
connector->connector_type = connector_type;
connector->connector_type_id =
ida_simple_get(connector_ida, 1, 0, GFP_KERNEL);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f0556e654116..e27ffa3561af 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -318,6 +318,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
list_add_tail(&crtc->head, &config->crtc_list);
crtc->index = config->num_crtc++;
+ /* we have 32bit crtc bitmasks */
+ WARN_ON(crtc->index >= 32);
+
crtc->primary = primary;
crtc->cursor = cursor;
if (primary && !primary->possible_crtcs)
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 59e0ebe733f8..66d0cdd217fa 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -136,6 +136,9 @@ int drm_encoder_init(struct drm_device *dev,
list_add_tail(&encoder->head, &dev->mode_config.encoder_list);
encoder->index = dev->mode_config.num_encoder++;
+ /* we have 32bit encoder bitmasks */
+ WARN_ON(encoder->index >= 32);
+
out_put:
if (ret)
drm_mode_object_unregister(dev, &encoder->base);
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 2c90519576a3..0a8d807603c1 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -242,6 +242,9 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
list_add_tail(&plane->head, &config->plane_list);
plane->index = config->num_total_plane++;
+ /* we have 32bit plane bitmasks */
+ WARN_ON(plane->index >= 32);
+
drm_object_attach_property(&plane->base,
config->plane_type_property,
plane->type);
--
2.13.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* ✗ Fi.CI.BAT: warning for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-24 18:37 [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
@ 2018-01-24 19:21 ` Patchwork
2018-01-24 21:01 ` [PATCH] " Harry Wentland
` (6 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-01-24 19:21 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
URL : https://patchwork.freedesktop.org/series/37058/
State : warning
== Summary ==
Series 37058v1 drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
https://patchwork.freedesktop.org/api/1.0/series/37058/revisions/1/mbox/
Test debugfs_test:
Subgroup read_all_entries:
pass -> DMESG-WARN (fi-elk-e7500) fdo#103989 +1
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass -> DMESG-WARN (fi-hsw-4770r)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
incomplete -> PASS (fi-bdw-5557u) fdo#104162
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#104162 https://bugs.freedesktop.org/show_bug.cgi?id=104162
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:419s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:428s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:373s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:488s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:280s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:483s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:483s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:466s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:456s
fi-elk-e7500 total:224 pass:168 dwarn:10 dfail:0 fail:0 skip:45
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:281s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:513s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:396s
fi-hsw-4770r total:288 pass:260 dwarn:1 dfail:0 fail:0 skip:27 time:401s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:409s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:459s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:411s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:461s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:501s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:454s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:506s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:581s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:429s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:504s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:531s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:489s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:489s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:421s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:429s
fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:522s
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:396s
Blacklisted hosts:
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:568s
fi-glk-dsi total:288 pass:183 dwarn:1 dfail:4 fail:0 skip:100 time:361s
d33d70e5449f1477b09de4963fc7397890da1263 drm-tip: 2018y-01m-24d-18h-16m-27s UTC integration manifest
2180375a38f7 drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7772/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-24 18:37 [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
2018-01-24 19:21 ` ✗ Fi.CI.BAT: warning for " Patchwork
@ 2018-01-24 21:01 ` Harry Wentland
2018-01-24 21:24 ` Ville Syrjälä
2018-01-24 21:47 ` [PATCH v2] " Ville Syrjala
` (5 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Harry Wentland @ 2018-01-24 21:01 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On 2018-01-24 01:37 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
> Naturally we can only do that if the index of those objects stays
> below 32. Issue a warning whenever we exceed that limit, hopefully
> prompting someone to fix the problem.
>
> Or should we just outright fail the object initializatio perhaps?
>
This would make sense in my opinion. index >= 32 looks like it would lead to completely undefined behavior when trying to attach objects to each other.
Harry
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/drm_connector.c | 3 +++
> drivers/gpu/drm/drm_crtc.c | 3 +++
> drivers/gpu/drm/drm_encoder.c | 3 +++
> drivers/gpu/drm/drm_plane.c | 3 +++
> 4 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b85a7749709d..9278a81c5d54 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -211,6 +211,9 @@ int drm_connector_init(struct drm_device *dev,
> connector->index = ret;
> ret = 0;
>
> + /* we have 32bit connector bitmasks */
> + WARN_ON(connector->index >= 32);
> +
> connector->connector_type = connector_type;
> connector->connector_type_id =
> ida_simple_get(connector_ida, 1, 0, GFP_KERNEL);
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f0556e654116..e27ffa3561af 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -318,6 +318,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> list_add_tail(&crtc->head, &config->crtc_list);
> crtc->index = config->num_crtc++;
>
> + /* we have 32bit crtc bitmasks */
> + WARN_ON(crtc->index >= 32);
> +
> crtc->primary = primary;
> crtc->cursor = cursor;
> if (primary && !primary->possible_crtcs)
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 59e0ebe733f8..66d0cdd217fa 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -136,6 +136,9 @@ int drm_encoder_init(struct drm_device *dev,
> list_add_tail(&encoder->head, &dev->mode_config.encoder_list);
> encoder->index = dev->mode_config.num_encoder++;
>
> + /* we have 32bit encoder bitmasks */
> + WARN_ON(encoder->index >= 32);
> +
> out_put:
> if (ret)
> drm_mode_object_unregister(dev, &encoder->base);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 2c90519576a3..0a8d807603c1 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -242,6 +242,9 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> list_add_tail(&plane->head, &config->plane_list);
> plane->index = config->num_total_plane++;
>
> + /* we have 32bit plane bitmasks */
> + WARN_ON(plane->index >= 32);
> +
> drm_object_attach_property(&plane->base,
> config->plane_type_property,
> plane->type);
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-24 21:01 ` [PATCH] " Harry Wentland
@ 2018-01-24 21:24 ` Ville Syrjälä
2018-01-24 21:26 ` Harry Wentland
2018-01-24 21:30 ` [Intel-gfx] " Ville Syrjälä
0 siblings, 2 replies; 18+ messages in thread
From: Ville Syrjälä @ 2018-01-24 21:24 UTC (permalink / raw)
To: Harry Wentland; +Cc: intel-gfx, dri-devel
On Wed, Jan 24, 2018 at 04:01:18PM -0500, Harry Wentland wrote:
> On 2018-01-24 01:37 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
> > Naturally we can only do that if the index of those objects stays
> > below 32. Issue a warning whenever we exceed that limit, hopefully
> > prompting someone to fix the problem.
> >
> > Or should we just outright fail the object initializatio perhaps?
> >
>
> This would make sense in my opinion. index >= 32 looks like it would lead to completely undefined behavior when trying to attach objects to each other.
I suppose. The counter argument is that this is basically the developer
shooting themselves in the foot intentionally, so it's a bit hard to
justify complicating the runtime error handling for this.
The connector case might be well justified though since the index there
comes from ida, and with MST connectors can come and go as they please.
So it might not be entirely impossible to overflow the bits there.
>
> Harry
>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/drm_connector.c | 3 +++
> > drivers/gpu/drm/drm_crtc.c | 3 +++
> > drivers/gpu/drm/drm_encoder.c | 3 +++
> > drivers/gpu/drm/drm_plane.c | 3 +++
> > 4 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index b85a7749709d..9278a81c5d54 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -211,6 +211,9 @@ int drm_connector_init(struct drm_device *dev,
> > connector->index = ret;
> > ret = 0;
> >
> > + /* we have 32bit connector bitmasks */
> > + WARN_ON(connector->index >= 32);
> > +
> > connector->connector_type = connector_type;
> > connector->connector_type_id =
> > ida_simple_get(connector_ida, 1, 0, GFP_KERNEL);
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index f0556e654116..e27ffa3561af 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -318,6 +318,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> > list_add_tail(&crtc->head, &config->crtc_list);
> > crtc->index = config->num_crtc++;
> >
> > + /* we have 32bit crtc bitmasks */
> > + WARN_ON(crtc->index >= 32);
> > +
> > crtc->primary = primary;
> > crtc->cursor = cursor;
> > if (primary && !primary->possible_crtcs)
> > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > index 59e0ebe733f8..66d0cdd217fa 100644
> > --- a/drivers/gpu/drm/drm_encoder.c
> > +++ b/drivers/gpu/drm/drm_encoder.c
> > @@ -136,6 +136,9 @@ int drm_encoder_init(struct drm_device *dev,
> > list_add_tail(&encoder->head, &dev->mode_config.encoder_list);
> > encoder->index = dev->mode_config.num_encoder++;
> >
> > + /* we have 32bit encoder bitmasks */
> > + WARN_ON(encoder->index >= 32);
> > +
> > out_put:
> > if (ret)
> > drm_mode_object_unregister(dev, &encoder->base);
> > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > index 2c90519576a3..0a8d807603c1 100644
> > --- a/drivers/gpu/drm/drm_plane.c
> > +++ b/drivers/gpu/drm/drm_plane.c
> > @@ -242,6 +242,9 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> > list_add_tail(&plane->head, &config->plane_list);
> > plane->index = config->num_total_plane++;
> >
> > + /* we have 32bit plane bitmasks */
> > + WARN_ON(plane->index >= 32);
> > +
> > drm_object_attach_property(&plane->base,
> > config->plane_type_property,
> > plane->type);
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-24 21:24 ` Ville Syrjälä
@ 2018-01-24 21:26 ` Harry Wentland
2018-01-24 21:30 ` [Intel-gfx] " Ville Syrjälä
1 sibling, 0 replies; 18+ messages in thread
From: Harry Wentland @ 2018-01-24 21:26 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On 2018-01-24 04:24 PM, Ville Syrjälä wrote:
> On Wed, Jan 24, 2018 at 04:01:18PM -0500, Harry Wentland wrote:
>> On 2018-01-24 01:37 PM, Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
>>> Naturally we can only do that if the index of those objects stays
>>> below 32. Issue a warning whenever we exceed that limit, hopefully
>>> prompting someone to fix the problem.
>>>
>>> Or should we just outright fail the object initializatio perhaps?
>>>
>>
>> This would make sense in my opinion. index >= 32 looks like it would lead to completely undefined behavior when trying to attach objects to each other.
>
> I suppose. The counter argument is that this is basically the developer
> shooting themselves in the foot intentionally, so it's a bit hard to
> justify complicating the runtime error handling for this.
>
True, if this needs extensive runtime error handling a WARN_ON is probably better.
> The connector case might be well justified though since the index there
> comes from ida, and with MST connectors can come and go as they please.
> So it might not be entirely impossible to overflow the bits there.
>
Right, I was trying to think of a real-life case where this might happen. MST is a good one (although still quite rare in the vast majority of cases).
Harry
>>
>> Harry
>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/drm_connector.c | 3 +++
>>> drivers/gpu/drm/drm_crtc.c | 3 +++
>>> drivers/gpu/drm/drm_encoder.c | 3 +++
>>> drivers/gpu/drm/drm_plane.c | 3 +++
>>> 4 files changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>>> index b85a7749709d..9278a81c5d54 100644
>>> --- a/drivers/gpu/drm/drm_connector.c
>>> +++ b/drivers/gpu/drm/drm_connector.c
>>> @@ -211,6 +211,9 @@ int drm_connector_init(struct drm_device *dev,
>>> connector->index = ret;
>>> ret = 0;
>>>
>>> + /* we have 32bit connector bitmasks */
>>> + WARN_ON(connector->index >= 32);
>>> +
>>> connector->connector_type = connector_type;
>>> connector->connector_type_id =
>>> ida_simple_get(connector_ida, 1, 0, GFP_KERNEL);
>>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>>> index f0556e654116..e27ffa3561af 100644
>>> --- a/drivers/gpu/drm/drm_crtc.c
>>> +++ b/drivers/gpu/drm/drm_crtc.c
>>> @@ -318,6 +318,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
>>> list_add_tail(&crtc->head, &config->crtc_list);
>>> crtc->index = config->num_crtc++;
>>>
>>> + /* we have 32bit crtc bitmasks */
>>> + WARN_ON(crtc->index >= 32);
>>> +
>>> crtc->primary = primary;
>>> crtc->cursor = cursor;
>>> if (primary && !primary->possible_crtcs)
>>> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
>>> index 59e0ebe733f8..66d0cdd217fa 100644
>>> --- a/drivers/gpu/drm/drm_encoder.c
>>> +++ b/drivers/gpu/drm/drm_encoder.c
>>> @@ -136,6 +136,9 @@ int drm_encoder_init(struct drm_device *dev,
>>> list_add_tail(&encoder->head, &dev->mode_config.encoder_list);
>>> encoder->index = dev->mode_config.num_encoder++;
>>>
>>> + /* we have 32bit encoder bitmasks */
>>> + WARN_ON(encoder->index >= 32);
>>> +
>>> out_put:
>>> if (ret)
>>> drm_mode_object_unregister(dev, &encoder->base);
>>> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>>> index 2c90519576a3..0a8d807603c1 100644
>>> --- a/drivers/gpu/drm/drm_plane.c
>>> +++ b/drivers/gpu/drm/drm_plane.c
>>> @@ -242,6 +242,9 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>>> list_add_tail(&plane->head, &config->plane_list);
>>> plane->index = config->num_total_plane++;
>>>
>>> + /* we have 32bit plane bitmasks */
>>> + WARN_ON(plane->index >= 32);
>>> +
>>> drm_object_attach_property(&plane->base,
>>> config->plane_type_property,
>>> plane->type);
>>>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Intel-gfx] [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-24 21:24 ` Ville Syrjälä
2018-01-24 21:26 ` Harry Wentland
@ 2018-01-24 21:30 ` Ville Syrjälä
1 sibling, 0 replies; 18+ messages in thread
From: Ville Syrjälä @ 2018-01-24 21:30 UTC (permalink / raw)
To: Harry Wentland; +Cc: intel-gfx, dri-devel
On Wed, Jan 24, 2018 at 11:24:05PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 24, 2018 at 04:01:18PM -0500, Harry Wentland wrote:
> > On 2018-01-24 01:37 PM, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
> > > Naturally we can only do that if the index of those objects stays
> > > below 32. Issue a warning whenever we exceed that limit, hopefully
> > > prompting someone to fix the problem.
> > >
> > > Or should we just outright fail the object initializatio perhaps?
> > >
> >
> > This would make sense in my opinion. index >= 32 looks like it would lead to completely undefined behavior when trying to attach objects to each other.
>
> I suppose. The counter argument is that this is basically the developer
> shooting themselves in the foot intentionally, so it's a bit hard to
> justify complicating the runtime error handling for this.
And after giving this one second of actual though I figured it won't
actually complicate anything if we do it early enough. These functions
can't be safely called concurrently anyway, and so we can ignore any
races that sort of thing would bring.
>
> The connector case might be well justified though since the index there
> comes from ida, and with MST connectors can come and go as they please.
> So it might not be entirely impossible to overflow the bits there.
>
> >
> > Harry
> >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/drm_connector.c | 3 +++
> > > drivers/gpu/drm/drm_crtc.c | 3 +++
> > > drivers/gpu/drm/drm_encoder.c | 3 +++
> > > drivers/gpu/drm/drm_plane.c | 3 +++
> > > 4 files changed, 12 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index b85a7749709d..9278a81c5d54 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -211,6 +211,9 @@ int drm_connector_init(struct drm_device *dev,
> > > connector->index = ret;
> > > ret = 0;
> > >
> > > + /* we have 32bit connector bitmasks */
> > > + WARN_ON(connector->index >= 32);
> > > +
> > > connector->connector_type = connector_type;
> > > connector->connector_type_id =
> > > ida_simple_get(connector_ida, 1, 0, GFP_KERNEL);
> > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > > index f0556e654116..e27ffa3561af 100644
> > > --- a/drivers/gpu/drm/drm_crtc.c
> > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > @@ -318,6 +318,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> > > list_add_tail(&crtc->head, &config->crtc_list);
> > > crtc->index = config->num_crtc++;
> > >
> > > + /* we have 32bit crtc bitmasks */
> > > + WARN_ON(crtc->index >= 32);
> > > +
> > > crtc->primary = primary;
> > > crtc->cursor = cursor;
> > > if (primary && !primary->possible_crtcs)
> > > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > > index 59e0ebe733f8..66d0cdd217fa 100644
> > > --- a/drivers/gpu/drm/drm_encoder.c
> > > +++ b/drivers/gpu/drm/drm_encoder.c
> > > @@ -136,6 +136,9 @@ int drm_encoder_init(struct drm_device *dev,
> > > list_add_tail(&encoder->head, &dev->mode_config.encoder_list);
> > > encoder->index = dev->mode_config.num_encoder++;
> > >
> > > + /* we have 32bit encoder bitmasks */
> > > + WARN_ON(encoder->index >= 32);
> > > +
> > > out_put:
> > > if (ret)
> > > drm_mode_object_unregister(dev, &encoder->base);
> > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > > index 2c90519576a3..0a8d807603c1 100644
> > > --- a/drivers/gpu/drm/drm_plane.c
> > > +++ b/drivers/gpu/drm/drm_plane.c
> > > @@ -242,6 +242,9 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> > > list_add_tail(&plane->head, &config->plane_list);
> > > plane->index = config->num_total_plane++;
> > >
> > > + /* we have 32bit plane bitmasks */
> > > + WARN_ON(plane->index >= 32);
> > > +
> > > drm_object_attach_property(&plane->base,
> > > config->plane_type_property,
> > > plane->type);
> > >
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-24 18:37 [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
2018-01-24 19:21 ` ✗ Fi.CI.BAT: warning for " Patchwork
2018-01-24 21:01 ` [PATCH] " Harry Wentland
@ 2018-01-24 21:47 ` Ville Syrjala
2018-01-25 9:17 ` Maarten Lankhorst
2018-01-24 22:30 ` ✓ Fi.CI.BAT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev2) Patchwork
` (4 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjala @ 2018-01-24 21:47 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Daniel Vetter
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
Naturally we can only do that if the index of those objects stays
below 32. Issue a warning whenever we exceed that limit, hopefully
prompting someone to fix the problem.
For connectors the issue is a bit more complicated as they can
be created/destroyed at runtime due to MST. So the problem is no
longer a purely theoretical programmer error. As the connector
indexes are allocated via ida, we can simply limit the maximum
value the ida is allowed to hand out. The error handling is already
in place.
v2: Return an error to the caller (Harry)
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_connector.c | 3 ++-
drivers/gpu/drm/drm_crtc.c | 4 ++++
drivers/gpu/drm/drm_encoder.c | 4 ++++
drivers/gpu/drm/drm_plane.c | 4 ++++
4 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b85a7749709d..f4a689ab990a 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -205,7 +205,8 @@ int drm_connector_init(struct drm_device *dev,
connector->dev = dev;
connector->funcs = funcs;
- ret = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL);
+ /* connector index is used with 32bit bitmasks */
+ ret = ida_simple_get(&config->connector_ida, 0, 32, GFP_KERNEL);
if (ret < 0)
goto out_put;
connector->index = ret;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f0556e654116..5b4be382a1d7 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -282,6 +282,10 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
WARN_ON(primary && primary->type != DRM_PLANE_TYPE_PRIMARY);
WARN_ON(cursor && cursor->type != DRM_PLANE_TYPE_CURSOR);
+ /* crtc index is used with 32bit bitmasks */
+ if (WARN_ON(config->num_crtc >= 32))
+ return -EINVAL;
+
crtc->dev = dev;
crtc->funcs = funcs;
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 59e0ebe733f8..273e1c59c54a 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -110,6 +110,10 @@ int drm_encoder_init(struct drm_device *dev,
{
int ret;
+ /* encoder index is used with 32bit bitmasks */
+ if (WARN_ON(dev->mode_config.num_encoder >= 32))
+ return -EINVAL;
+
ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 2c90519576a3..22b54663b6e7 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -173,6 +173,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
unsigned int format_modifier_count = 0;
int ret;
+ /* plane index is used with 32bit bitmasks */
+ if (WARN_ON(config->num_total_plane >= 32))
+ return -EINVAL;
+
ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
if (ret)
return ret;
--
2.13.6
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev2)
2018-01-24 18:37 [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
` (2 preceding siblings ...)
2018-01-24 21:47 ` [PATCH v2] " Ville Syrjala
@ 2018-01-24 22:30 ` Patchwork
2018-01-25 1:54 ` ✓ Fi.CI.IGT: " Patchwork
` (3 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-01-24 22:30 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev2)
URL : https://patchwork.freedesktop.org/series/37058/
State : success
== Summary ==
Series 37058v2 drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
https://patchwork.freedesktop.org/api/1.0/series/37058/revisions/2/mbox/
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:426s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:438s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:373s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:495s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:282s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:491s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:487s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:472s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:459s
fi-elk-e7500 total:224 pass:168 dwarn:10 dfail:0 fail:0 skip:45
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:295s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:518s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:395s
fi-hsw-4770r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:401s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:415s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:453s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:414s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:460s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:495s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:453s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:505s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:583s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:429s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:513s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:537s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:490s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:495s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:419s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:431s
fi-snb-2520m total:245 pass:211 dwarn:0 dfail:0 fail:0 skip:33
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:401s
Blacklisted hosts:
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:571s
fi-glk-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:477s
9d8467fe5626095314bc34449457798dae066fbb drm-tip: 2018y-01m-24d-19h-59m-41s UTC integration manifest
1c7a8e2421cd drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7774/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✓ Fi.CI.IGT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev2)
2018-01-24 18:37 [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
` (3 preceding siblings ...)
2018-01-24 22:30 ` ✓ Fi.CI.BAT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev2) Patchwork
@ 2018-01-25 1:54 ` Patchwork
2018-01-25 13:30 ` [PATCH v3] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
` (2 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-01-25 1:54 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev2)
URL : https://patchwork.freedesktop.org/series/37058/
State : success
== Summary ==
Test kms_plane:
Subgroup plane-panning-bottom-right-suspend-pipe-b-planes:
pass -> FAIL (shard-hsw) fdo#103540
Test kms_flip:
Subgroup 2x-flip-vs-modeset-interruptible:
dmesg-warn -> PASS (shard-hsw)
Subgroup plain-flip-fb-recreate-interruptible:
fail -> PASS (shard-hsw) fdo#100368
Test kms_frontbuffer_tracking:
Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
pass -> FAIL (shard-snb) fdo#101623 +1
Test drv_suspend:
Subgroup forcewake-hibernate:
fail -> SKIP (shard-snb) fdo#103375
Test kms_cursor_legacy:
Subgroup flip-vs-cursor-crc-atomic:
fail -> PASS (shard-apl)
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
shard-apl total:2838 pass:1753 dwarn:1 dfail:0 fail:22 skip:1062 time:12529s
shard-hsw total:2838 pass:1733 dwarn:1 dfail:0 fail:13 skip:1090 time:11853s
shard-snb total:2838 pass:1328 dwarn:1 dfail:0 fail:11 skip:1498 time:6626s
Blacklisted hosts:
shard-kbl total:2740 pass:1799 dwarn:9 dfail:1 fail:25 skip:905 time:9131s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7774/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-24 21:47 ` [PATCH v2] " Ville Syrjala
@ 2018-01-25 9:17 ` Maarten Lankhorst
2018-01-25 11:27 ` Ville Syrjälä
0 siblings, 1 reply; 18+ messages in thread
From: Maarten Lankhorst @ 2018-01-25 9:17 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: Daniel Vetter, intel-gfx
Op 24-01-18 om 22:47 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
> Naturally we can only do that if the index of those objects stays
> below 32. Issue a warning whenever we exceed that limit, hopefully
> prompting someone to fix the problem.
>
> For connectors the issue is a bit more complicated as they can
> be created/destroyed at runtime due to MST. So the problem is no
> longer a purely theoretical programmer error. As the connector
> indexes are allocated via ida, we can simply limit the maximum
> value the ida is allowed to hand out. The error handling is already
> in place.
>
> v2: Return an error to the caller (Harry)
>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/drm_connector.c | 3 ++-
> drivers/gpu/drm/drm_crtc.c | 4 ++++
> drivers/gpu/drm/drm_encoder.c | 4 ++++
> drivers/gpu/drm/drm_plane.c | 4 ++++
> 4 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b85a7749709d..f4a689ab990a 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -205,7 +205,8 @@ int drm_connector_init(struct drm_device *dev,
> connector->dev = dev;
> connector->funcs = funcs;
>
> - ret = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL);
> + /* connector index is used with 32bit bitmasks */
> + ret = ida_simple_get(&config->connector_ida, 0, 32, GFP_KERNEL);
> if (ret < 0)
> goto out_put;
> connector->index = ret;
Could we also put a DRM_DEBUG_KMS in here if allocation fails? Since it's more likely to happen now..
Otherwise looks good so with that change:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f0556e654116..5b4be382a1d7 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -282,6 +282,10 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> WARN_ON(primary && primary->type != DRM_PLANE_TYPE_PRIMARY);
> WARN_ON(cursor && cursor->type != DRM_PLANE_TYPE_CURSOR);
>
> + /* crtc index is used with 32bit bitmasks */
> + if (WARN_ON(config->num_crtc >= 32))
> + return -EINVAL;
> +
> crtc->dev = dev;
> crtc->funcs = funcs;
>
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 59e0ebe733f8..273e1c59c54a 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -110,6 +110,10 @@ int drm_encoder_init(struct drm_device *dev,
> {
> int ret;
>
> + /* encoder index is used with 32bit bitmasks */
> + if (WARN_ON(dev->mode_config.num_encoder >= 32))
> + return -EINVAL;
> +
> ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 2c90519576a3..22b54663b6e7 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -173,6 +173,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> unsigned int format_modifier_count = 0;
> int ret;
>
> + /* plane index is used with 32bit bitmasks */
> + if (WARN_ON(config->num_total_plane >= 32))
> + return -EINVAL;
> +
> ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
> if (ret)
> return ret;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-25 9:17 ` Maarten Lankhorst
@ 2018-01-25 11:27 ` Ville Syrjälä
2018-01-25 13:25 ` [Intel-gfx] " Ville Syrjälä
0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2018-01-25 11:27 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Daniel Vetter, intel-gfx, dri-devel
On Thu, Jan 25, 2018 at 10:17:21AM +0100, Maarten Lankhorst wrote:
> Op 24-01-18 om 22:47 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
> > Naturally we can only do that if the index of those objects stays
> > below 32. Issue a warning whenever we exceed that limit, hopefully
> > prompting someone to fix the problem.
> >
> > For connectors the issue is a bit more complicated as they can
> > be created/destroyed at runtime due to MST. So the problem is no
> > longer a purely theoretical programmer error. As the connector
> > indexes are allocated via ida, we can simply limit the maximum
> > value the ida is allowed to hand out. The error handling is already
> > in place.
> >
> > v2: Return an error to the caller (Harry)
> >
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/drm_connector.c | 3 ++-
> > drivers/gpu/drm/drm_crtc.c | 4 ++++
> > drivers/gpu/drm/drm_encoder.c | 4 ++++
> > drivers/gpu/drm/drm_plane.c | 4 ++++
> > 4 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index b85a7749709d..f4a689ab990a 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -205,7 +205,8 @@ int drm_connector_init(struct drm_device *dev,
> > connector->dev = dev;
> > connector->funcs = funcs;
> >
> > - ret = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL);
> > + /* connector index is used with 32bit bitmasks */
> > + ret = ida_simple_get(&config->connector_ida, 0, 32, GFP_KERNEL);
> > if (ret < 0)
> > goto out_put;
> > connector->index = ret;
> Could we also put a DRM_DEBUG_KMS in here if allocation fails? Since it's more likely to happen now..
I was actually pondering using DRM_ERROR(), or maybe even WARN(). It
shouldn't be directly user triggerable so I think we want to get the
bugreport if we do run out of bits.
>
> Otherwise looks good so with that change:
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index f0556e654116..5b4be382a1d7 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -282,6 +282,10 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> > WARN_ON(primary && primary->type != DRM_PLANE_TYPE_PRIMARY);
> > WARN_ON(cursor && cursor->type != DRM_PLANE_TYPE_CURSOR);
> >
> > + /* crtc index is used with 32bit bitmasks */
> > + if (WARN_ON(config->num_crtc >= 32))
> > + return -EINVAL;
> > +
> > crtc->dev = dev;
> > crtc->funcs = funcs;
> >
> > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > index 59e0ebe733f8..273e1c59c54a 100644
> > --- a/drivers/gpu/drm/drm_encoder.c
> > +++ b/drivers/gpu/drm/drm_encoder.c
> > @@ -110,6 +110,10 @@ int drm_encoder_init(struct drm_device *dev,
> > {
> > int ret;
> >
> > + /* encoder index is used with 32bit bitmasks */
> > + if (WARN_ON(dev->mode_config.num_encoder >= 32))
> > + return -EINVAL;
> > +
> > ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
> > if (ret)
> > return ret;
> > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > index 2c90519576a3..22b54663b6e7 100644
> > --- a/drivers/gpu/drm/drm_plane.c
> > +++ b/drivers/gpu/drm/drm_plane.c
> > @@ -173,6 +173,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> > unsigned int format_modifier_count = 0;
> > int ret;
> >
> > + /* plane index is used with 32bit bitmasks */
> > + if (WARN_ON(config->num_total_plane >= 32))
> > + return -EINVAL;
> > +
> > ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
> > if (ret)
> > return ret;
>
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-25 11:27 ` Ville Syrjälä
@ 2018-01-25 13:25 ` Ville Syrjälä
0 siblings, 0 replies; 18+ messages in thread
From: Ville Syrjälä @ 2018-01-25 13:25 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Daniel Vetter, intel-gfx, dri-devel
On Thu, Jan 25, 2018 at 01:27:27PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 25, 2018 at 10:17:21AM +0100, Maarten Lankhorst wrote:
> > Op 24-01-18 om 22:47 schreef Ville Syrjala:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
> > > Naturally we can only do that if the index of those objects stays
> > > below 32. Issue a warning whenever we exceed that limit, hopefully
> > > prompting someone to fix the problem.
> > >
> > > For connectors the issue is a bit more complicated as they can
> > > be created/destroyed at runtime due to MST. So the problem is no
> > > longer a purely theoretical programmer error. As the connector
> > > indexes are allocated via ida, we can simply limit the maximum
> > > value the ida is allowed to hand out. The error handling is already
> > > in place.
> > >
> > > v2: Return an error to the caller (Harry)
> > >
> > > Cc: Harry Wentland <harry.wentland@amd.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/drm_connector.c | 3 ++-
> > > drivers/gpu/drm/drm_crtc.c | 4 ++++
> > > drivers/gpu/drm/drm_encoder.c | 4 ++++
> > > drivers/gpu/drm/drm_plane.c | 4 ++++
> > > 4 files changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index b85a7749709d..f4a689ab990a 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -205,7 +205,8 @@ int drm_connector_init(struct drm_device *dev,
> > > connector->dev = dev;
> > > connector->funcs = funcs;
> > >
> > > - ret = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL);
> > > + /* connector index is used with 32bit bitmasks */
> > > + ret = ida_simple_get(&config->connector_ida, 0, 32, GFP_KERNEL);
> > > if (ret < 0)
> > > goto out_put;
> > > connector->index = ret;
> > Could we also put a DRM_DEBUG_KMS in here if allocation fails? Since it's more likely to happen now..
>
> I was actually pondering using DRM_ERROR(), or maybe even WARN(). It
> shouldn't be directly user triggerable so I think we want to get the
> bugreport if we do run out of bits.
OTOH we are likely to get the "my screen don't work" bugreport anyway,
so maybe a debug print is enough.
>
> >
> > Otherwise looks good so with that change:
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > > index f0556e654116..5b4be382a1d7 100644
> > > --- a/drivers/gpu/drm/drm_crtc.c
> > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > @@ -282,6 +282,10 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> > > WARN_ON(primary && primary->type != DRM_PLANE_TYPE_PRIMARY);
> > > WARN_ON(cursor && cursor->type != DRM_PLANE_TYPE_CURSOR);
> > >
> > > + /* crtc index is used with 32bit bitmasks */
> > > + if (WARN_ON(config->num_crtc >= 32))
> > > + return -EINVAL;
> > > +
> > > crtc->dev = dev;
> > > crtc->funcs = funcs;
> > >
> > > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > > index 59e0ebe733f8..273e1c59c54a 100644
> > > --- a/drivers/gpu/drm/drm_encoder.c
> > > +++ b/drivers/gpu/drm/drm_encoder.c
> > > @@ -110,6 +110,10 @@ int drm_encoder_init(struct drm_device *dev,
> > > {
> > > int ret;
> > >
> > > + /* encoder index is used with 32bit bitmasks */
> > > + if (WARN_ON(dev->mode_config.num_encoder >= 32))
> > > + return -EINVAL;
> > > +
> > > ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
> > > if (ret)
> > > return ret;
> > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > > index 2c90519576a3..22b54663b6e7 100644
> > > --- a/drivers/gpu/drm/drm_plane.c
> > > +++ b/drivers/gpu/drm/drm_plane.c
> > > @@ -173,6 +173,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> > > unsigned int format_modifier_count = 0;
> > > int ret;
> > >
> > > + /* plane index is used with 32bit bitmasks */
> > > + if (WARN_ON(config->num_total_plane >= 32))
> > > + return -EINVAL;
> > > +
> > > ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
> > > if (ret)
> > > return ret;
> >
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-24 18:37 [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
` (4 preceding siblings ...)
2018-01-25 1:54 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-01-25 13:30 ` Ville Syrjala
2018-01-25 15:12 ` Harry Wentland
2018-01-25 13:49 ` ✓ Fi.CI.BAT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3) Patchwork
2018-01-25 15:17 ` ✗ Fi.CI.IGT: failure " Patchwork
7 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjala @ 2018-01-25 13:30 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Harry Wentland, Daniel Vetter
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
Naturally we can only do that if the index of those objects stays
below 32. Issue a warning whenever we exceed that limit, hopefully
prompting someone to fix the problem.
For connectors the issue is a bit more complicated as they can
be created/destroyed at runtime due to MST. So the problem is no
longer a purely theoretical programmer error. As the connector
indexes are allocated via ida, we can simply limit the maximum
value the ida is allowed to hand out. The error handling is already
in place.
v2: Return an error to the caller (Harry)
v3: Print a debug message so that we know what happened (Maarten)
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_connector.c | 9 +++++++--
drivers/gpu/drm/drm_crtc.c | 4 ++++
drivers/gpu/drm/drm_encoder.c | 4 ++++
drivers/gpu/drm/drm_plane.c | 4 ++++
4 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b85a7749709d..16b9c3810af2 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -205,9 +205,14 @@ int drm_connector_init(struct drm_device *dev,
connector->dev = dev;
connector->funcs = funcs;
- ret = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL);
- if (ret < 0)
+ /* connector index is used with 32bit bitmasks */
+ ret = ida_simple_get(&config->connector_ida, 0, 32, GFP_KERNEL);
+ if (ret < 0) {
+ DRM_DEBUG_KMS("Failed to allocate %s connector index: %d\n",
+ drm_connector_enum_list[connector_type].name,
+ ret);
goto out_put;
+ }
connector->index = ret;
ret = 0;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f0556e654116..5b4be382a1d7 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -282,6 +282,10 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
WARN_ON(primary && primary->type != DRM_PLANE_TYPE_PRIMARY);
WARN_ON(cursor && cursor->type != DRM_PLANE_TYPE_CURSOR);
+ /* crtc index is used with 32bit bitmasks */
+ if (WARN_ON(config->num_crtc >= 32))
+ return -EINVAL;
+
crtc->dev = dev;
crtc->funcs = funcs;
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 59e0ebe733f8..273e1c59c54a 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -110,6 +110,10 @@ int drm_encoder_init(struct drm_device *dev,
{
int ret;
+ /* encoder index is used with 32bit bitmasks */
+ if (WARN_ON(dev->mode_config.num_encoder >= 32))
+ return -EINVAL;
+
ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 2c90519576a3..22b54663b6e7 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -173,6 +173,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
unsigned int format_modifier_count = 0;
int ret;
+ /* plane index is used with 32bit bitmasks */
+ if (WARN_ON(config->num_total_plane >= 32))
+ return -EINVAL;
+
ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
if (ret)
return ret;
--
2.13.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3)
2018-01-24 18:37 [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
` (5 preceding siblings ...)
2018-01-25 13:30 ` [PATCH v3] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
@ 2018-01-25 13:49 ` Patchwork
2018-01-25 15:17 ` ✗ Fi.CI.IGT: failure " Patchwork
7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-01-25 13:49 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3)
URL : https://patchwork.freedesktop.org/series/37058/
State : success
== Summary ==
Series 37058v3 drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
https://patchwork.freedesktop.org/api/1.0/series/37058/revisions/3/mbox/
Test debugfs_test:
Subgroup read_all_entries:
dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989 +2
pass -> INCOMPLETE (fi-snb-2520m) fdo#103713
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:422s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:425s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:372s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:495s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:282s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:485s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:496s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:469s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:460s
fi-elk-e7500 total:229 pass:172 dwarn:9 dfail:1 fail:0 skip:46
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:280s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:515s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:402s
fi-hsw-4770r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:401s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:416s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:464s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:417s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:461s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:499s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:456s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:506s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:65 time:585s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:430s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:514s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:530s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:488s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:489s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:417s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:434s
fi-snb-2520m total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:399s
Blacklisted hosts:
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:570s
fi-glk-dsi total:288 pass:188 dwarn:1 dfail:4 fail:0 skip:95 time:389s
9d8467fe5626095314bc34449457798dae066fbb drm-tip: 2018y-01m-24d-19h-59m-41s UTC integration manifest
02addb97fd0d drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7776/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-25 13:30 ` [PATCH v3] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
@ 2018-01-25 15:12 ` Harry Wentland
2018-01-29 17:21 ` Ville Syrjälä
0 siblings, 1 reply; 18+ messages in thread
From: Harry Wentland @ 2018-01-25 15:12 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: Daniel Vetter, intel-gfx
On 2018-01-25 08:30 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
> Naturally we can only do that if the index of those objects stays
> below 32. Issue a warning whenever we exceed that limit, hopefully
> prompting someone to fix the problem.
>
> For connectors the issue is a bit more complicated as they can
> be created/destroyed at runtime due to MST. So the problem is no
> longer a purely theoretical programmer error. As the connector
> indexes are allocated via ida, we can simply limit the maximum
> value the ida is allowed to hand out. The error handling is already
> in place.
>
> v2: Return an error to the caller (Harry)
> v3: Print a debug message so that we know what happened (Maarten)
>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Harry
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/drm_connector.c | 9 +++++++--
> drivers/gpu/drm/drm_crtc.c | 4 ++++
> drivers/gpu/drm/drm_encoder.c | 4 ++++
> drivers/gpu/drm/drm_plane.c | 4 ++++
> 4 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b85a7749709d..16b9c3810af2 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -205,9 +205,14 @@ int drm_connector_init(struct drm_device *dev,
> connector->dev = dev;
> connector->funcs = funcs;
>
> - ret = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL);
> - if (ret < 0)
> + /* connector index is used with 32bit bitmasks */
> + ret = ida_simple_get(&config->connector_ida, 0, 32, GFP_KERNEL);
> + if (ret < 0) {
> + DRM_DEBUG_KMS("Failed to allocate %s connector index: %d\n",
> + drm_connector_enum_list[connector_type].name,
> + ret);
> goto out_put;
> + }
> connector->index = ret;
> ret = 0;
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f0556e654116..5b4be382a1d7 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -282,6 +282,10 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> WARN_ON(primary && primary->type != DRM_PLANE_TYPE_PRIMARY);
> WARN_ON(cursor && cursor->type != DRM_PLANE_TYPE_CURSOR);
>
> + /* crtc index is used with 32bit bitmasks */
> + if (WARN_ON(config->num_crtc >= 32))
> + return -EINVAL;
> +
> crtc->dev = dev;
> crtc->funcs = funcs;
>
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 59e0ebe733f8..273e1c59c54a 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -110,6 +110,10 @@ int drm_encoder_init(struct drm_device *dev,
> {
> int ret;
>
> + /* encoder index is used with 32bit bitmasks */
> + if (WARN_ON(dev->mode_config.num_encoder >= 32))
> + return -EINVAL;
> +
> ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 2c90519576a3..22b54663b6e7 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -173,6 +173,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> unsigned int format_modifier_count = 0;
> int ret;
>
> + /* plane index is used with 32bit bitmasks */
> + if (WARN_ON(config->num_total_plane >= 32))
> + return -EINVAL;
> +
> ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
> if (ret)
> return ret;
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✗ Fi.CI.IGT: failure for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3)
2018-01-24 18:37 [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
` (6 preceding siblings ...)
2018-01-25 13:49 ` ✓ Fi.CI.BAT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3) Patchwork
@ 2018-01-25 15:17 ` Patchwork
2018-01-29 16:45 ` Ville Syrjälä
7 siblings, 1 reply; 18+ messages in thread
From: Patchwork @ 2018-01-25 15:17 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3)
URL : https://patchwork.freedesktop.org/series/37058/
State : failure
== Summary ==
Test drv_suspend:
Subgroup fence-restore-untiled-hibernate:
fail -> SKIP (shard-snb) fdo#103375
Test perf:
Subgroup oa-exponents:
pass -> FAIL (shard-apl) fdo#102254
Test kms_flip:
Subgroup plain-flip-ts-check-interruptible:
pass -> FAIL (shard-hsw) fdo#100368 +1
Subgroup 2x-flip-vs-modeset-interruptible:
dmesg-warn -> PASS (shard-hsw) fdo#102614
Subgroup 2x-flip-vs-expired-vblank:
pass -> FAIL (shard-hsw) fdo#102887
Test kms_plane_lowres:
Subgroup pipe-a-tiling-x:
pass -> FAIL (shard-apl)
Test kms_cursor_legacy:
Subgroup 2x-long-flip-vs-cursor-legacy:
pass -> FAIL (shard-hsw)
Subgroup flip-vs-cursor-crc-atomic:
fail -> PASS (shard-apl) fdo#102670
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
shard-apl total:2838 pass:1751 dwarn:1 dfail:0 fail:24 skip:1062 time:12644s
shard-hsw total:2838 pass:1732 dwarn:1 dfail:0 fail:14 skip:1090 time:11915s
shard-snb total:2838 pass:1330 dwarn:1 dfail:0 fail:9 skip:1498 time:6646s
Blacklisted hosts:
shard-kbl total:2838 pass:1861 dwarn:13 dfail:1 fail:23 skip:940 time:9553s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7776/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: ✗ Fi.CI.IGT: failure for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3)
2018-01-25 15:17 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-01-29 16:45 ` Ville Syrjälä
0 siblings, 0 replies; 18+ messages in thread
From: Ville Syrjälä @ 2018-01-29 16:45 UTC (permalink / raw)
To: intel-gfx
On Thu, Jan 25, 2018 at 03:17:21PM -0000, Patchwork wrote:
> == Series Details ==
>
> Series: drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3)
> URL : https://patchwork.freedesktop.org/series/37058/
> State : failure
>
> == Summary ==
>
> Test drv_suspend:
> Subgroup fence-restore-untiled-hibernate:
> fail -> SKIP (shard-snb) fdo#103375
> Test perf:
> Subgroup oa-exponents:
> pass -> FAIL (shard-apl) fdo#102254
> Test kms_flip:
> Subgroup plain-flip-ts-check-interruptible:
> pass -> FAIL (shard-hsw) fdo#100368 +1
> Subgroup 2x-flip-vs-modeset-interruptible:
> dmesg-warn -> PASS (shard-hsw) fdo#102614
> Subgroup 2x-flip-vs-expired-vblank:
> pass -> FAIL (shard-hsw) fdo#102887
> Test kms_plane_lowres:
> Subgroup pipe-a-tiling-x:
> pass -> FAIL (shard-apl)
CRC mismatch of some sort:
(kms_plane_lowres:1487) igt-debugfs-CRITICAL: Test assertion failure function igt_assert_crc_equal, file igt_debugfs.c:356:
(kms_plane_lowres:1487) igt-debugfs-CRITICAL: Failed assertion: !mismatch
(kms_plane_lowres:1487) igt-debugfs-CRITICAL: Last errno: 9, Bad file descriptor
> Test kms_cursor_legacy:
> Subgroup 2x-long-flip-vs-cursor-legacy:
> pass -> FAIL (shard-hsw)
Missed vbl seq:
(kms_cursor_legacy:10485) CRITICAL: Test assertion failure function two_screens_flip_vs_cursor, file kms_cursor_legacy.c:976:
(kms_cursor_legacy:10485) CRITICAL: Failed assertion: vbl.sequence == vblank_start + 1
(kms_cursor_legacy:10485) CRITICAL: Last errno: 25, Inappropriate ioctl
for device
(kms_cursor_legacy:10485) CRITICAL: error: 38169 != 38168
Both seem unrelated to the patch.
> Subgroup flip-vs-cursor-crc-atomic:
> fail -> PASS (shard-apl) fdo#102670
>
> fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
> fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
> fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
> fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
> fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
> fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
>
> shard-apl total:2838 pass:1751 dwarn:1 dfail:0 fail:24 skip:1062 time:12644s
> shard-hsw total:2838 pass:1732 dwarn:1 dfail:0 fail:14 skip:1090 time:11915s
> shard-snb total:2838 pass:1330 dwarn:1 dfail:0 fail:9 skip:1498 time:6646s
> Blacklisted hosts:
> shard-kbl total:2838 pass:1861 dwarn:13 dfail:1 fail:23 skip:940 time:9553s
>
> == Logs ==
>
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7776/shards.html
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks
2018-01-25 15:12 ` Harry Wentland
@ 2018-01-29 17:21 ` Ville Syrjälä
0 siblings, 0 replies; 18+ messages in thread
From: Ville Syrjälä @ 2018-01-29 17:21 UTC (permalink / raw)
To: Harry Wentland; +Cc: Daniel Vetter, intel-gfx, dri-devel
On Thu, Jan 25, 2018 at 10:12:52AM -0500, Harry Wentland wrote:
> On 2018-01-25 08:30 AM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We use 32bit bitmasks to track planes/crtcs/encoders/connectors.
> > Naturally we can only do that if the index of those objects stays
> > below 32. Issue a warning whenever we exceed that limit, hopefully
> > prompting someone to fix the problem.
> >
> > For connectors the issue is a bit more complicated as they can
> > be created/destroyed at runtime due to MST. So the problem is no
> > longer a purely theoretical programmer error. As the connector
> > indexes are allocated via ida, we can simply limit the maximum
> > value the ida is allowed to hand out. The error handling is already
> > in place.
> >
> > v2: Return an error to the caller (Harry)
> > v3: Print a debug message so that we know what happened (Maarten)
> >
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Thanks for the review. Pushed to drm-misc-next.
>
> Harry
>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/drm_connector.c | 9 +++++++--
> > drivers/gpu/drm/drm_crtc.c | 4 ++++
> > drivers/gpu/drm/drm_encoder.c | 4 ++++
> > drivers/gpu/drm/drm_plane.c | 4 ++++
> > 4 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index b85a7749709d..16b9c3810af2 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -205,9 +205,14 @@ int drm_connector_init(struct drm_device *dev,
> > connector->dev = dev;
> > connector->funcs = funcs;
> >
> > - ret = ida_simple_get(&config->connector_ida, 0, 0, GFP_KERNEL);
> > - if (ret < 0)
> > + /* connector index is used with 32bit bitmasks */
> > + ret = ida_simple_get(&config->connector_ida, 0, 32, GFP_KERNEL);
> > + if (ret < 0) {
> > + DRM_DEBUG_KMS("Failed to allocate %s connector index: %d\n",
> > + drm_connector_enum_list[connector_type].name,
> > + ret);
> > goto out_put;
> > + }
> > connector->index = ret;
> > ret = 0;
> >
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index f0556e654116..5b4be382a1d7 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -282,6 +282,10 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> > WARN_ON(primary && primary->type != DRM_PLANE_TYPE_PRIMARY);
> > WARN_ON(cursor && cursor->type != DRM_PLANE_TYPE_CURSOR);
> >
> > + /* crtc index is used with 32bit bitmasks */
> > + if (WARN_ON(config->num_crtc >= 32))
> > + return -EINVAL;
> > +
> > crtc->dev = dev;
> > crtc->funcs = funcs;
> >
> > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > index 59e0ebe733f8..273e1c59c54a 100644
> > --- a/drivers/gpu/drm/drm_encoder.c
> > +++ b/drivers/gpu/drm/drm_encoder.c
> > @@ -110,6 +110,10 @@ int drm_encoder_init(struct drm_device *dev,
> > {
> > int ret;
> >
> > + /* encoder index is used with 32bit bitmasks */
> > + if (WARN_ON(dev->mode_config.num_encoder >= 32))
> > + return -EINVAL;
> > +
> > ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
> > if (ret)
> > return ret;
> > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > index 2c90519576a3..22b54663b6e7 100644
> > --- a/drivers/gpu/drm/drm_plane.c
> > +++ b/drivers/gpu/drm/drm_plane.c
> > @@ -173,6 +173,10 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> > unsigned int format_modifier_count = 0;
> > int ret;
> >
> > + /* plane index is used with 32bit bitmasks */
> > + if (WARN_ON(config->num_total_plane >= 32))
> > + return -EINVAL;
> > +
> > ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
> > if (ret)
> > return ret;
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-01-29 17:21 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-24 18:37 [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
2018-01-24 19:21 ` ✗ Fi.CI.BAT: warning for " Patchwork
2018-01-24 21:01 ` [PATCH] " Harry Wentland
2018-01-24 21:24 ` Ville Syrjälä
2018-01-24 21:26 ` Harry Wentland
2018-01-24 21:30 ` [Intel-gfx] " Ville Syrjälä
2018-01-24 21:47 ` [PATCH v2] " Ville Syrjala
2018-01-25 9:17 ` Maarten Lankhorst
2018-01-25 11:27 ` Ville Syrjälä
2018-01-25 13:25 ` [Intel-gfx] " Ville Syrjälä
2018-01-24 22:30 ` ✓ Fi.CI.BAT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev2) Patchwork
2018-01-25 1:54 ` ✓ Fi.CI.IGT: " Patchwork
2018-01-25 13:30 ` [PATCH v3] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks Ville Syrjala
2018-01-25 15:12 ` Harry Wentland
2018-01-29 17:21 ` Ville Syrjälä
2018-01-25 13:49 ` ✓ Fi.CI.BAT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3) Patchwork
2018-01-25 15:17 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-01-29 16:45 ` Ville Syrjälä
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.