[PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[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.

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

✗ Fi.CI.BAT: warning for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[Patchwork]

== 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

Re: [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[Harry Wentland]

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

Re: [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[Ville Syrjälä]

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

Re: [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[Harry Wentland]

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

Re: [Intel-gfx] [PATCH] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[Ville Syrjälä]

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

[PATCH v2] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[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;
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

✓ Fi.CI.BAT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev2)

[Patchwork]

== 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

✓ Fi.CI.IGT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev2)

[Patchwork]

== 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

Re: [PATCH v2] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[Maarten Lankhorst]

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

Re: [PATCH v2] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[Ville Syrjälä]

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

Re: [Intel-gfx] [PATCH v2] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[Ville Syrjälä]

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

[PATCH v3] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[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)
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

✓ Fi.CI.BAT: success for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3)

[Patchwork]

== 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

Re: [PATCH v3] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[Harry Wentland]

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

✗ Fi.CI.IGT: failure for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3)

[Patchwork]

== 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

Re: ✗ Fi.CI.IGT: failure for drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks (rev3)

[Ville Syrjälä]

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

Re: [PATCH v3] drm: Warn if plane/crtc/encoder/connector index exceeds our 32bit bitmasks

[Ville Syrjälä]

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