All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: Don't oops in drm_calc_timestamping_constants() if drm_vblank_init() wasn't called
@ 2015-11-12 12:34 ville.syrjala
  2015-11-12 14:12 ` Jeff Moyer
  2015-11-12 15:02 ` Alex Deucher
  0 siblings, 2 replies; 3+ messages in thread
From: ville.syrjala @ 2015-11-12 12:34 UTC (permalink / raw)
  To: dri-devel; +Cc: Jeff Moyer

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Seems the crtc helpers call drm_calc_timestamping_constants()
unconditionally even if the driver didn't initialize vblank support by
calling drm_vblank_init(). That used to be OK since the constants were
stored under drm_crtc.

However I broke this with
commit eba1f35dfe14 ("drm: Move timestamping constants into drm_vblank_crtc")
when I moved the constants to live inside the drm_vblank_crtc struct
instead. If drm_vblank_init() isn't called, we don't allocate these
structures, and so drm_calc_timestamping_constants() will oops.

Fix it by adding a check into drm_calc_timestamping_constants() to see
if vblank support was initialized at all. And to keep in line with other
such checks, also toss in a check and warn for the case where vblank
support was initialized, but the wrong number of crtcs was specified.

Fixes the following sort of oops:
 BUG: unable to handle kernel NULL pointer dereference at 00000000000000b0
 IP: [<ffffffffa014b266>] drm_calc_timestamping_constants+0x86/0x130 [drm]
 PGD 0
 Oops: 0002 [#1] SMP
 Modules linked in: sr_mod cdrom mgag200(+) i2c_algo_bit drm_kms_helper ahci syscopyarea sysfillrect sysimgblt libahci fb_sys_fops bnx2x ttm tg3(+) mdio drm ptp sd_mod libata i2c_core pps_core libcrc32c hpsa dm_mirror dm_region_hash dm_log dm_mod
 CPU: 0 PID: 418 Comm: kworker/0:2 Not tainted 4.3.0+ #1
 Hardware name: HP ProLiant DL380 Gen9, BIOS P89 06/09/2015
 Workqueue: events work_for_cpu_fn
 task: ffff88046ca95500 ti: ffff88007830c000 task.ti: ffff88007830c000
 RIP: 0010:[<ffffffffa014b266>]  [<ffffffffa014b266>] drm_calc_timestamping_constants+0x86/0x130 [drm]
 RSP: 0018:ffff88007830f4e8  EFLAGS: 00010246
 RAX: 0000000000fe4c00 RBX: ffff88006a849160 RCX: 0000000000000540
 RDX: 0000000000000000 RSI: 000000000000fde8 RDI: ffff88006a849000
 RBP: ffff88007830f518 R08: ffff88007830c000 R09: 00000001b87e3712
 R10: 00000000000050c4 R11: 0000000000000000 R12: 0000000000fe4c00
 R13: ffff88006a849000 R14: 0000000000000000 R15: 000000000000fde8
 FS:  0000000000000000(0000) GS:ffff88046f800000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00000000000000b0 CR3: 00000000019d6000 CR4: 00000000001406f0
 Stack:
  ffff88007830f518 ffff88006a849000 ffff880c69b90340 ffff880c69b90000
  ffff880c69b90348 ffff880c69b90340 ffff88007830f748 ffffffffa042f7e7
  ffff88006a849090 0000000000000000 ffff88006a849160 0000000000000000
 Call Trace:
  [<ffffffffa042f7e7>] drm_crtc_helper_set_mode+0x3d7/0x4b0 [drm_kms_helper]
  [<ffffffffa04307d4>] drm_crtc_helper_set_config+0x8d4/0xb10 [drm_kms_helper]
  [<ffffffffa01548d4>] drm_mode_set_config_internal+0x64/0x100 [drm]
  [<ffffffffa043c342>] drm_fb_helper_pan_display+0xa2/0x280 [drm_kms_helper]
  [<ffffffff81392c7b>] fb_pan_display+0xbb/0x170
  [<ffffffff8138cf70>] bit_update_start+0x20/0x50
  [<ffffffff8138b81b>] fbcon_switch+0x39b/0x590
  [<ffffffff8140a3d0>] redraw_screen+0x1a0/0x240
  [<ffffffff8140b30e>] do_bind_con_driver+0x2ee/0x310
  [<ffffffff8140b651>] do_take_over_console+0x141/0x1b0
  [<ffffffff81387377>] do_fbcon_takeover+0x57/0xb0
  [<ffffffff8138c98b>] fbcon_event_notify+0x60b/0x750
  [<ffffffff810a5599>] notifier_call_chain+0x49/0x70
  [<ffffffff810a58dd>] __blocking_notifier_call_chain+0x4d/0x70
  [<ffffffff810a5916>] blocking_notifier_call_chain+0x16/0x20
  [<ffffffff8139282b>] fb_notifier_call_chain+0x1b/0x20
  [<ffffffff81394881>] register_framebuffer+0x1f1/0x330
  [<ffffffffa043d9aa>] drm_fb_helper_initial_config+0x27a/0x3d0 [drm_kms_helper]
  [<ffffffffa0469b4d>] mgag200_fbdev_init+0xdd/0xf0 [mgag200]
  [<ffffffffa0468586>] mgag200_modeset_init+0x176/0x1e0 [mgag200]
  [<ffffffffa0464659>] mgag200_driver_load+0x3f9/0x580 [mgag200]
  [<ffffffffa014e067>] drm_dev_register+0xa7/0xb0 [drm]
  [<ffffffffa015054f>] drm_get_pci_dev+0x8f/0x1e0 [drm]
  [<ffffffffa046937b>] mga_pci_probe+0x9b/0xc0 [mgag200]
  [<ffffffff813662d5>] local_pci_probe+0x45/0xa0
  [<ffffffff8109afe4>] work_for_cpu_fn+0x14/0x20
  [<ffffffff8109e13c>] process_one_work+0x14c/0x3c0
  [<ffffffff8109eaa4>] worker_thread+0x244/0x470
  [<ffffffff8168bfba>] ? __schedule+0x2aa/0x760
  [<ffffffff8109e860>] ? rescuer_thread+0x310/0x310
  [<ffffffff810a4438>] kthread+0xd8/0xf0
  [<ffffffff810a4360>] ? kthread_park+0x60/0x60
  [<ffffffff8169030f>] ret_from_fork+0x3f/0x70
  [<ffffffff810a4360>] ? kthread_park+0x60/0x60
 Code: f6 31 d2 41 89 c2 8b 83 b4 00 00 00 0f af c1 48 98 48 69 c0 40 42 0f 00 48 f7 f6 f6 43 74 10 41 89 c4 75 26 f6 05 9a 6f 03 00 01 <45> 89 96 b0 00 00 00 45 89 a6 ac 00 00 00 75 35 48 83 c4 08 5b
 RIP  [<ffffffffa014b266>] drm_calc_timestamping_constants+0x86/0x130 [drm]
  RSP <ffff88007830f4e8>
 CR2: 00000000000000b0

Cc: Jeff Moyer <jmoyer@redhat.com>
Reported-by: Jeff Moyer <jmoyer@redhat.com>
References: http://lists.freedesktop.org/archives/dri-devel/2015-November/094217.html
Fixes: eba1f35dfe14 ("drm: Move timestamping constants into drm_vblank_crtc")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_irq.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index eba6337..2151ea5 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -616,10 +616,18 @@ int drm_control(struct drm_device *dev, void *data,
 void drm_calc_timestamping_constants(struct drm_crtc *crtc,
 				     const struct drm_display_mode *mode)
 {
-	struct drm_vblank_crtc *vblank = &crtc->dev->vblank[drm_crtc_index(crtc)];
+	struct drm_device *dev = crtc->dev;
+	unsigned int pipe = drm_crtc_index(crtc);
+	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
 	int linedur_ns = 0, framedur_ns = 0;
 	int dotclock = mode->crtc_clock;
 
+	if (!dev->num_crtcs)
+		return;
+
+	if (WARN_ON(pipe >= dev->num_crtcs))
+		return;
+
 	/* Valid dotclock? */
 	if (dotclock > 0) {
 		int frame_size = mode->crtc_htotal * mode->crtc_vtotal;
-- 
2.4.10

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm: Don't oops in drm_calc_timestamping_constants() if drm_vblank_init() wasn't called
  2015-11-12 12:34 [PATCH] drm: Don't oops in drm_calc_timestamping_constants() if drm_vblank_init() wasn't called ville.syrjala
@ 2015-11-12 14:12 ` Jeff Moyer
  2015-11-12 15:02 ` Alex Deucher
  1 sibling, 0 replies; 3+ messages in thread
From: Jeff Moyer @ 2015-11-12 14:12 UTC (permalink / raw)
  To: ville.syrjala; +Cc: dri-devel

ville.syrjala@linux.intel.com writes:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Seems the crtc helpers call drm_calc_timestamping_constants()
> unconditionally even if the driver didn't initialize vblank support by
> calling drm_vblank_init(). That used to be OK since the constants were
> stored under drm_crtc.
>
> However I broke this with
> commit eba1f35dfe14 ("drm: Move timestamping constants into drm_vblank_crtc")
> when I moved the constants to live inside the drm_vblank_crtc struct
> instead. If drm_vblank_init() isn't called, we don't allocate these
> structures, and so drm_calc_timestamping_constants() will oops.
>
> Fix it by adding a check into drm_calc_timestamping_constants() to see
> if vblank support was initialized at all. And to keep in line with other
> such checks, also toss in a check and warn for the case where vblank
> support was initialized, but the wrong number of crtcs was specified.

That was fast, thanks!  Works for me.

Tested-by: Jeff Moyer <jmoyer@redhat.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm: Don't oops in drm_calc_timestamping_constants() if drm_vblank_init() wasn't called
  2015-11-12 12:34 [PATCH] drm: Don't oops in drm_calc_timestamping_constants() if drm_vblank_init() wasn't called ville.syrjala
  2015-11-12 14:12 ` Jeff Moyer
@ 2015-11-12 15:02 ` Alex Deucher
  1 sibling, 0 replies; 3+ messages in thread
From: Alex Deucher @ 2015-11-12 15:02 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Jeff Moyer, Maling list - DRI developers

On Thu, Nov 12, 2015 at 7:34 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Seems the crtc helpers call drm_calc_timestamping_constants()
> unconditionally even if the driver didn't initialize vblank support by
> calling drm_vblank_init(). That used to be OK since the constants were
> stored under drm_crtc.
>
> However I broke this with
> commit eba1f35dfe14 ("drm: Move timestamping constants into drm_vblank_crtc")
> when I moved the constants to live inside the drm_vblank_crtc struct
> instead. If drm_vblank_init() isn't called, we don't allocate these
> structures, and so drm_calc_timestamping_constants() will oops.
>
> Fix it by adding a check into drm_calc_timestamping_constants() to see
> if vblank support was initialized at all. And to keep in line with other
> such checks, also toss in a check and warn for the case where vblank
> support was initialized, but the wrong number of crtcs was specified.
>
> Fixes the following sort of oops:
>  BUG: unable to handle kernel NULL pointer dereference at 00000000000000b0
>  IP: [<ffffffffa014b266>] drm_calc_timestamping_constants+0x86/0x130 [drm]
>  PGD 0
>  Oops: 0002 [#1] SMP
>  Modules linked in: sr_mod cdrom mgag200(+) i2c_algo_bit drm_kms_helper ahci syscopyarea sysfillrect sysimgblt libahci fb_sys_fops bnx2x ttm tg3(+) mdio drm ptp sd_mod libata i2c_core pps_core libcrc32c hpsa dm_mirror dm_region_hash dm_log dm_mod
>  CPU: 0 PID: 418 Comm: kworker/0:2 Not tainted 4.3.0+ #1
>  Hardware name: HP ProLiant DL380 Gen9, BIOS P89 06/09/2015
>  Workqueue: events work_for_cpu_fn
>  task: ffff88046ca95500 ti: ffff88007830c000 task.ti: ffff88007830c000
>  RIP: 0010:[<ffffffffa014b266>]  [<ffffffffa014b266>] drm_calc_timestamping_constants+0x86/0x130 [drm]
>  RSP: 0018:ffff88007830f4e8  EFLAGS: 00010246
>  RAX: 0000000000fe4c00 RBX: ffff88006a849160 RCX: 0000000000000540
>  RDX: 0000000000000000 RSI: 000000000000fde8 RDI: ffff88006a849000
>  RBP: ffff88007830f518 R08: ffff88007830c000 R09: 00000001b87e3712
>  R10: 00000000000050c4 R11: 0000000000000000 R12: 0000000000fe4c00
>  R13: ffff88006a849000 R14: 0000000000000000 R15: 000000000000fde8
>  FS:  0000000000000000(0000) GS:ffff88046f800000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 00000000000000b0 CR3: 00000000019d6000 CR4: 00000000001406f0
>  Stack:
>   ffff88007830f518 ffff88006a849000 ffff880c69b90340 ffff880c69b90000
>   ffff880c69b90348 ffff880c69b90340 ffff88007830f748 ffffffffa042f7e7
>   ffff88006a849090 0000000000000000 ffff88006a849160 0000000000000000
>  Call Trace:
>   [<ffffffffa042f7e7>] drm_crtc_helper_set_mode+0x3d7/0x4b0 [drm_kms_helper]
>   [<ffffffffa04307d4>] drm_crtc_helper_set_config+0x8d4/0xb10 [drm_kms_helper]
>   [<ffffffffa01548d4>] drm_mode_set_config_internal+0x64/0x100 [drm]
>   [<ffffffffa043c342>] drm_fb_helper_pan_display+0xa2/0x280 [drm_kms_helper]
>   [<ffffffff81392c7b>] fb_pan_display+0xbb/0x170
>   [<ffffffff8138cf70>] bit_update_start+0x20/0x50
>   [<ffffffff8138b81b>] fbcon_switch+0x39b/0x590
>   [<ffffffff8140a3d0>] redraw_screen+0x1a0/0x240
>   [<ffffffff8140b30e>] do_bind_con_driver+0x2ee/0x310
>   [<ffffffff8140b651>] do_take_over_console+0x141/0x1b0
>   [<ffffffff81387377>] do_fbcon_takeover+0x57/0xb0
>   [<ffffffff8138c98b>] fbcon_event_notify+0x60b/0x750
>   [<ffffffff810a5599>] notifier_call_chain+0x49/0x70
>   [<ffffffff810a58dd>] __blocking_notifier_call_chain+0x4d/0x70
>   [<ffffffff810a5916>] blocking_notifier_call_chain+0x16/0x20
>   [<ffffffff8139282b>] fb_notifier_call_chain+0x1b/0x20
>   [<ffffffff81394881>] register_framebuffer+0x1f1/0x330
>   [<ffffffffa043d9aa>] drm_fb_helper_initial_config+0x27a/0x3d0 [drm_kms_helper]
>   [<ffffffffa0469b4d>] mgag200_fbdev_init+0xdd/0xf0 [mgag200]
>   [<ffffffffa0468586>] mgag200_modeset_init+0x176/0x1e0 [mgag200]
>   [<ffffffffa0464659>] mgag200_driver_load+0x3f9/0x580 [mgag200]
>   [<ffffffffa014e067>] drm_dev_register+0xa7/0xb0 [drm]
>   [<ffffffffa015054f>] drm_get_pci_dev+0x8f/0x1e0 [drm]
>   [<ffffffffa046937b>] mga_pci_probe+0x9b/0xc0 [mgag200]
>   [<ffffffff813662d5>] local_pci_probe+0x45/0xa0
>   [<ffffffff8109afe4>] work_for_cpu_fn+0x14/0x20
>   [<ffffffff8109e13c>] process_one_work+0x14c/0x3c0
>   [<ffffffff8109eaa4>] worker_thread+0x244/0x470
>   [<ffffffff8168bfba>] ? __schedule+0x2aa/0x760
>   [<ffffffff8109e860>] ? rescuer_thread+0x310/0x310
>   [<ffffffff810a4438>] kthread+0xd8/0xf0
>   [<ffffffff810a4360>] ? kthread_park+0x60/0x60
>   [<ffffffff8169030f>] ret_from_fork+0x3f/0x70
>   [<ffffffff810a4360>] ? kthread_park+0x60/0x60
>  Code: f6 31 d2 41 89 c2 8b 83 b4 00 00 00 0f af c1 48 98 48 69 c0 40 42 0f 00 48 f7 f6 f6 43 74 10 41 89 c4 75 26 f6 05 9a 6f 03 00 01 <45> 89 96 b0 00 00 00 45 89 a6 ac 00 00 00 75 35 48 83 c4 08 5b
>  RIP  [<ffffffffa014b266>] drm_calc_timestamping_constants+0x86/0x130 [drm]
>   RSP <ffff88007830f4e8>
>  CR2: 00000000000000b0
>
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Reported-by: Jeff Moyer <jmoyer@redhat.com>
> References: http://lists.freedesktop.org/archives/dri-devel/2015-November/094217.html
> Fixes: eba1f35dfe14 ("drm: Move timestamping constants into drm_vblank_crtc")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/drm_irq.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index eba6337..2151ea5 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -616,10 +616,18 @@ int drm_control(struct drm_device *dev, void *data,
>  void drm_calc_timestamping_constants(struct drm_crtc *crtc,
>                                      const struct drm_display_mode *mode)
>  {
> -       struct drm_vblank_crtc *vblank = &crtc->dev->vblank[drm_crtc_index(crtc)];
> +       struct drm_device *dev = crtc->dev;
> +       unsigned int pipe = drm_crtc_index(crtc);
> +       struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>         int linedur_ns = 0, framedur_ns = 0;
>         int dotclock = mode->crtc_clock;
>
> +       if (!dev->num_crtcs)
> +               return;
> +
> +       if (WARN_ON(pipe >= dev->num_crtcs))
> +               return;
> +
>         /* Valid dotclock? */
>         if (dotclock > 0) {
>                 int frame_size = mode->crtc_htotal * mode->crtc_vtotal;
> --
> 2.4.10
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-11-12 15:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-12 12:34 [PATCH] drm: Don't oops in drm_calc_timestamping_constants() if drm_vblank_init() wasn't called ville.syrjala
2015-11-12 14:12 ` Jeff Moyer
2015-11-12 15:02 ` Alex Deucher

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.