* [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations
@ 2017-06-22 16:02 Chris Wilson
2017-06-22 16:18 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Chris Wilson @ 2017-06-22 16:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Commit fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer
flushes") adds a dependency to ifbdev->vma when flushing the framebufer,
but the checks are only against the existence of the ifbdev->fb and not
against ifbdev->vma. This leaves a window of opportunity where we may
try to operate on the fbdev prior to it being probed (thanks to
asynchronous booting).
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101534
Fixes: fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer flushes")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/i915/intel_fbdev.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 03347c6ae599..0c4cde6b2e6f 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -535,13 +535,14 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
drm_fb_helper_fini(&ifbdev->helper);
- if (ifbdev->fb) {
+ if (ifbdev->vma) {
mutex_lock(&ifbdev->helper.dev->struct_mutex);
intel_unpin_fb_vma(ifbdev->vma);
mutex_unlock(&ifbdev->helper.dev->struct_mutex);
+ }
+ if (ifbdev->fb)
drm_framebuffer_remove(&ifbdev->fb->base);
- }
kfree(ifbdev);
}
@@ -765,7 +766,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
struct intel_fbdev *ifbdev = dev_priv->fbdev;
struct fb_info *info;
- if (!ifbdev || !ifbdev->fb)
+ if (!ifbdev || !ifbdev->vma)
return;
info = ifbdev->helper.fbdev;
@@ -812,7 +813,7 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev)
{
struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
- if (ifbdev && ifbdev->fb)
+ if (ifbdev && ifbdev->vma)
drm_fb_helper_hotplug_event(&ifbdev->helper);
}
@@ -824,7 +825,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
return;
intel_fbdev_sync(ifbdev);
- if (!ifbdev->fb)
+ if (!ifbdev->vma)
return;
if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper) == 0)
--
2.13.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/fbdev: Check for existence of ifbdev->vma before operations
2017-06-22 16:02 [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations Chris Wilson
@ 2017-06-22 16:18 ` Patchwork
2017-06-28 11:18 ` [PATCH] " Chris Wilson
2017-07-04 9:59 ` Tvrtko Ursulin
2 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2017-06-22 16:18 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/fbdev: Check for existence of ifbdev->vma before operations
URL : https://patchwork.freedesktop.org/series/26235/
State : success
== Summary ==
Series 26235v1 drm/i915/fbdev: Check for existence of ifbdev->vma before operations
https://patchwork.freedesktop.org/api/1.0/series/26235/revisions/1/mbox/
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass -> FAIL (fi-snb-2600) fdo#100007
Test prime_busy:
Subgroup basic-wait-after-default:
dmesg-warn -> PASS (fi-skl-6700hq) fdo#101515 +2
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#101515 https://bugs.freedesktop.org/show_bug.cgi?id=101515
fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:445s
fi-bdw-gvtdvm total:279 pass:257 dwarn:8 dfail:0 fail:0 skip:14 time:427s
fi-bsw-n3050 total:279 pass:242 dwarn:1 dfail:0 fail:0 skip:36 time:533s
fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:503s
fi-byt-j1900 total:279 pass:253 dwarn:2 dfail:0 fail:0 skip:24 time:484s
fi-byt-n2820 total:279 pass:249 dwarn:2 dfail:0 fail:0 skip:28 time:480s
fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:587s
fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:432s
fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:416s
fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:416s
fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:496s
fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:483s
fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:467s
fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:568s
fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:578s
fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:455s
fi-skl-6700hq total:279 pass:222 dwarn:2 dfail:0 fail:30 skip:24 time:341s
fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:463s
fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:488s
fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:448s
fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:545s
fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:409s
781051b4174a922661593ecf853618bcbd4ecf6c drm-tip: 2017y-06m-22d-13h-14m-47s UTC integration manifest
781eca5 drm/i915/fbdev: Check for existence of ifbdev->vma before operations
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5028/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations
2017-06-22 16:02 [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations Chris Wilson
2017-06-22 16:18 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-06-28 11:18 ` Chris Wilson
2017-06-29 12:09 ` Chris Wilson
2017-07-04 9:59 ` Tvrtko Ursulin
2 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2017-06-28 11:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Quoting Chris Wilson (2017-06-22 17:02:11)
> Commit fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer
> flushes") adds a dependency to ifbdev->vma when flushing the framebufer,
> but the checks are only against the existence of the ifbdev->fb and not
> against ifbdev->vma. This leaves a window of opportunity where we may
> try to operate on the fbdev prior to it being probed (thanks to
> asynchronous booting).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101534
> Fixes: fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer flushes")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
Ping?
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 03347c6ae599..0c4cde6b2e6f 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -535,13 +535,14 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
>
> drm_fb_helper_fini(&ifbdev->helper);
>
> - if (ifbdev->fb) {
> + if (ifbdev->vma) {
> mutex_lock(&ifbdev->helper.dev->struct_mutex);
> intel_unpin_fb_vma(ifbdev->vma);
> mutex_unlock(&ifbdev->helper.dev->struct_mutex);
> + }
>
> + if (ifbdev->fb)
> drm_framebuffer_remove(&ifbdev->fb->base);
> - }
>
> kfree(ifbdev);
> }
> @@ -765,7 +766,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
> struct intel_fbdev *ifbdev = dev_priv->fbdev;
> struct fb_info *info;
>
> - if (!ifbdev || !ifbdev->fb)
> + if (!ifbdev || !ifbdev->vma)
> return;
>
> info = ifbdev->helper.fbdev;
> @@ -812,7 +813,7 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev)
> {
> struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
>
> - if (ifbdev && ifbdev->fb)
> + if (ifbdev && ifbdev->vma)
> drm_fb_helper_hotplug_event(&ifbdev->helper);
> }
>
> @@ -824,7 +825,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
> return;
>
> intel_fbdev_sync(ifbdev);
> - if (!ifbdev->fb)
> + if (!ifbdev->vma)
> return;
>
> if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper) == 0)
> --
> 2.13.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations
2017-06-28 11:18 ` [PATCH] " Chris Wilson
@ 2017-06-29 12:09 ` Chris Wilson
2017-07-03 9:46 ` Chris Wilson
0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2017-06-29 12:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Quoting Chris Wilson (2017-06-28 12:18:44)
> Quoting Chris Wilson (2017-06-22 17:02:11)
> > Commit fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer
> > flushes") adds a dependency to ifbdev->vma when flushing the framebufer,
> > but the checks are only against the existence of the ifbdev->fb and not
> > against ifbdev->vma. This leaves a window of opportunity where we may
> > try to operate on the fbdev prior to it being probed (thanks to
> > asynchronous booting).
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101534
> > Fixes: fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer flushes")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
>
> Ping?
Double ping for a boot time panic recently reintroduced?
> > ---
> > drivers/gpu/drm/i915/intel_fbdev.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > index 03347c6ae599..0c4cde6b2e6f 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -535,13 +535,14 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
> >
> > drm_fb_helper_fini(&ifbdev->helper);
> >
> > - if (ifbdev->fb) {
> > + if (ifbdev->vma) {
> > mutex_lock(&ifbdev->helper.dev->struct_mutex);
> > intel_unpin_fb_vma(ifbdev->vma);
> > mutex_unlock(&ifbdev->helper.dev->struct_mutex);
> > + }
> >
> > + if (ifbdev->fb)
> > drm_framebuffer_remove(&ifbdev->fb->base);
> > - }
> >
> > kfree(ifbdev);
> > }
> > @@ -765,7 +766,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
> > struct intel_fbdev *ifbdev = dev_priv->fbdev;
> > struct fb_info *info;
> >
> > - if (!ifbdev || !ifbdev->fb)
> > + if (!ifbdev || !ifbdev->vma)
> > return;
> >
> > info = ifbdev->helper.fbdev;
> > @@ -812,7 +813,7 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev)
> > {
> > struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
> >
> > - if (ifbdev && ifbdev->fb)
> > + if (ifbdev && ifbdev->vma)
> > drm_fb_helper_hotplug_event(&ifbdev->helper);
> > }
> >
> > @@ -824,7 +825,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
> > return;
> >
> > intel_fbdev_sync(ifbdev);
> > - if (!ifbdev->fb)
> > + if (!ifbdev->vma)
> > return;
> >
> > if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper) == 0)
> > --
> > 2.13.1
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations
2017-06-29 12:09 ` Chris Wilson
@ 2017-07-03 9:46 ` Chris Wilson
0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2017-07-03 9:46 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Quoting Chris Wilson (2017-06-29 13:09:31)
> Quoting Chris Wilson (2017-06-28 12:18:44)
> > Quoting Chris Wilson (2017-06-22 17:02:11)
> > > Commit fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer
> > > flushes") adds a dependency to ifbdev->vma when flushing the framebufer,
> > > but the checks are only against the existence of the ifbdev->fb and not
> > > against ifbdev->vma. This leaves a window of opportunity where we may
> > > try to operate on the fbdev prior to it being probed (thanks to
> > > asynchronous booting).
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101534
> > > Fixes: fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer flushes")
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> >
> > Ping?
>
> Double ping for a boot time panic recently reintroduced?
Another report of a boot time panic,
Tested-by: Takashi Iwai <tiwai@suse.de>
> > > ---
> > > drivers/gpu/drm/i915/intel_fbdev.c | 11 ++++++-----
> > > 1 file changed, 6 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > > index 03347c6ae599..0c4cde6b2e6f 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > @@ -535,13 +535,14 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
> > >
> > > drm_fb_helper_fini(&ifbdev->helper);
> > >
> > > - if (ifbdev->fb) {
> > > + if (ifbdev->vma) {
> > > mutex_lock(&ifbdev->helper.dev->struct_mutex);
> > > intel_unpin_fb_vma(ifbdev->vma);
> > > mutex_unlock(&ifbdev->helper.dev->struct_mutex);
> > > + }
> > >
> > > + if (ifbdev->fb)
> > > drm_framebuffer_remove(&ifbdev->fb->base);
> > > - }
> > >
> > > kfree(ifbdev);
> > > }
> > > @@ -765,7 +766,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
> > > struct intel_fbdev *ifbdev = dev_priv->fbdev;
> > > struct fb_info *info;
> > >
> > > - if (!ifbdev || !ifbdev->fb)
> > > + if (!ifbdev || !ifbdev->vma)
> > > return;
> > >
> > > info = ifbdev->helper.fbdev;
> > > @@ -812,7 +813,7 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev)
> > > {
> > > struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
> > >
> > > - if (ifbdev && ifbdev->fb)
> > > + if (ifbdev && ifbdev->vma)
> > > drm_fb_helper_hotplug_event(&ifbdev->helper);
> > > }
> > >
> > > @@ -824,7 +825,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
> > > return;
> > >
> > > intel_fbdev_sync(ifbdev);
> > > - if (!ifbdev->fb)
> > > + if (!ifbdev->vma)
> > > return;
> > >
> > > if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper) == 0)
> > > --
> > > 2.13.1
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations
2017-06-22 16:02 [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations Chris Wilson
2017-06-22 16:18 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-06-28 11:18 ` [PATCH] " Chris Wilson
@ 2017-07-04 9:59 ` Tvrtko Ursulin
2017-07-04 10:06 ` Chris Wilson
2 siblings, 1 reply; 9+ messages in thread
From: Tvrtko Ursulin @ 2017-07-04 9:59 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Daniel Vetter
On 22/06/2017 17:02, Chris Wilson wrote:
> Commit fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer
> flushes") adds a dependency to ifbdev->vma when flushing the framebufer,
> but the checks are only against the existence of the ifbdev->fb and not
> against ifbdev->vma. This leaves a window of opportunity where we may
> try to operate on the fbdev prior to it being probed (thanks to
> asynchronous booting).
How about changing the intelfb_alloc to not write the fb to ifbdev but
instead return it to the caller, so intelfb_create could then set the
ifbdev->fb and ifbdev->vma atomically under the mutex?
Regards,
Tvrtko
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101534
> Fixes: fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer flushes")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 03347c6ae599..0c4cde6b2e6f 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -535,13 +535,14 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
>
> drm_fb_helper_fini(&ifbdev->helper);
>
> - if (ifbdev->fb) {
> + if (ifbdev->vma) {
> mutex_lock(&ifbdev->helper.dev->struct_mutex);
> intel_unpin_fb_vma(ifbdev->vma);
> mutex_unlock(&ifbdev->helper.dev->struct_mutex);
> + }
>
> + if (ifbdev->fb)
> drm_framebuffer_remove(&ifbdev->fb->base);
> - }
>
> kfree(ifbdev);
> }
> @@ -765,7 +766,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
> struct intel_fbdev *ifbdev = dev_priv->fbdev;
> struct fb_info *info;
>
> - if (!ifbdev || !ifbdev->fb)
> + if (!ifbdev || !ifbdev->vma)
> return;
>
> info = ifbdev->helper.fbdev;
> @@ -812,7 +813,7 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev)
> {
> struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
>
> - if (ifbdev && ifbdev->fb)
> + if (ifbdev && ifbdev->vma)
> drm_fb_helper_hotplug_event(&ifbdev->helper);
> }
>
> @@ -824,7 +825,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
> return;
>
> intel_fbdev_sync(ifbdev);
> - if (!ifbdev->fb)
> + if (!ifbdev->vma)
> return;
>
> if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper) == 0)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations
2017-07-04 9:59 ` Tvrtko Ursulin
@ 2017-07-04 10:06 ` Chris Wilson
2017-07-04 11:45 ` Tvrtko Ursulin
0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2017-07-04 10:06 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: Daniel Vetter
Quoting Tvrtko Ursulin (2017-07-04 10:59:29)
>
> On 22/06/2017 17:02, Chris Wilson wrote:
> > Commit fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer
> > flushes") adds a dependency to ifbdev->vma when flushing the framebufer,
> > but the checks are only against the existence of the ifbdev->fb and not
> > against ifbdev->vma. This leaves a window of opportunity where we may
> > try to operate on the fbdev prior to it being probed (thanks to
> > asynchronous booting).
>
> How about changing the intelfb_alloc to not write the fb to ifbdev but
> instead return it to the caller, so intelfb_create could then set the
> ifbdev->fb and ifbdev->vma atomically under the mutex?
Midlayer mishap.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations
2017-07-04 10:06 ` Chris Wilson
@ 2017-07-04 11:45 ` Tvrtko Ursulin
2017-07-04 12:16 ` Chris Wilson
0 siblings, 1 reply; 9+ messages in thread
From: Tvrtko Ursulin @ 2017-07-04 11:45 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Daniel Vetter
On 04/07/2017 11:06, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-07-04 10:59:29)
>>
>> On 22/06/2017 17:02, Chris Wilson wrote:
>>> Commit fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer
>>> flushes") adds a dependency to ifbdev->vma when flushing the framebufer,
>>> but the checks are only against the existence of the ifbdev->fb and not
>>> against ifbdev->vma. This leaves a window of opportunity where we may
>>> try to operate on the fbdev prior to it being probed (thanks to
>>> asynchronous booting).
>>
>> How about changing the intelfb_alloc to not write the fb to ifbdev but
>> instead return it to the caller, so intelfb_create could then set the
>> ifbdev->fb and ifbdev->vma atomically under the mutex?
>
> Midlayer mishap.
After an IRC chat I see that the issue is intel_fbdev_init_bios also
sets the fb and not vma, which I missed earlier.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations
2017-07-04 11:45 ` Tvrtko Ursulin
@ 2017-07-04 12:16 ` Chris Wilson
0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2017-07-04 12:16 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: Daniel Vetter
Quoting Tvrtko Ursulin (2017-07-04 12:45:59)
>
> On 04/07/2017 11:06, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-07-04 10:59:29)
> >>
> >> On 22/06/2017 17:02, Chris Wilson wrote:
> >>> Commit fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer
> >>> flushes") adds a dependency to ifbdev->vma when flushing the framebufer,
> >>> but the checks are only against the existence of the ifbdev->fb and not
> >>> against ifbdev->vma. This leaves a window of opportunity where we may
> >>> try to operate on the fbdev prior to it being probed (thanks to
> >>> asynchronous booting).
> >>
> >> How about changing the intelfb_alloc to not write the fb to ifbdev but
> >> instead return it to the caller, so intelfb_create could then set the
> >> ifbdev->fb and ifbdev->vma atomically under the mutex?
> >
> > Midlayer mishap.
>
> After an IRC chat I see that the issue is intel_fbdev_init_bios also
> sets the fb and not vma, which I missed earlier.
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
I'll take that, and push with a cc: stable@
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-07-04 12:17 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-22 16:02 [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations Chris Wilson
2017-06-22 16:18 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-06-28 11:18 ` [PATCH] " Chris Wilson
2017-06-29 12:09 ` Chris Wilson
2017-07-03 9:46 ` Chris Wilson
2017-07-04 9:59 ` Tvrtko Ursulin
2017-07-04 10:06 ` Chris Wilson
2017-07-04 11:45 ` Tvrtko Ursulin
2017-07-04 12:16 ` Chris Wilson
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.