| Message ID | 20240927203946.695934-2-lyude@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vblank: Require a driver register vblank support for 0 or all CRTCs | expand |
Hello,
kernel test robot noticed "BUG:kernel_NULL_pointer_dereference,address" on:
commit: 8e1a430cf308254a61a2317a0dfc4d8f4b3e13cb ("[PATCH] drm/vblank: Require a driver register vblank support for 0 or all CRTCs")
url: https://github.com/intel-lab-lkp/linux/commits/Lyude-Paul/drm-vblank-Require-a-driver-register-vblank-support-for-0-or-all-CRTCs/20240928-044210
patch link: https://lore.kernel.org/all/20240927203946.695934-2-lyude@redhat.com/
patch subject: [PATCH] drm/vblank: Require a driver register vblank support for 0 or all CRTCs
in testcase: boot
compiler: gcc-12
test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 4G
(please refer to attached dmesg/kmsg for entire log/backtrace)
+---------------------------------------------+------------+------------+
| | 22512c3ee0 | 8e1a430cf3 |
+---------------------------------------------+------------+------------+
| boot_successes | 12 | 0 |
| boot_failures | 0 | 12 |
| BUG:kernel_NULL_pointer_dereference,address | 0 | 12 |
| Oops:Oops:#[##] | 0 | 12 |
| EIP:drm_vblank_init | 0 | 12 |
| Kernel_panic-not_syncing:Fatal_exception | 0 | 12 |
+---------------------------------------------+------------+------------+
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202410101418.5704b4a5-lkp@intel.com
[ 4.727010][ T1] BUG: kernel NULL pointer dereference, address: 00000188
[ 4.728324][ T1] #PF: supervisor read access in kernel mode
[ 4.729456][ T1] #PF: error_code(0x0000) - not-present page
[ 4.729853][ T1] *pdpt = 0000000000000000 *pde = f000ff53f000ff53
[ 4.729853][ T1] Oops: Oops: 0000 [#1]
[ 4.729853][ T1] CPU: 0 UID: 0 PID: 1 Comm: swapper Tainted: G T 6.11.0-rc7-01372-g8e1a430cf308 #1 577dd3e1adc1bccd6f381550d3179686c5f157a0
[ 4.729853][ T1] Tainted: [T]=RANDSTRUCT
[ 4.729853][ T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 4.729853][ T1] EIP: drm_vblank_init (drivers/gpu/drm/drm_vblank.c:534)
[ 4.729853][ T1] Code: 89 c6 53 83 ec 08 89 55 ec 8b 90 64 05 00 00 39 d1 74 56 8d 42 f8 eb 12 90 8b 5a 04 85 db 74 17 8b 50 08 8d 42 f8 39 d1 74 3f <8b> 90 90 01 00 00 8b 7a 08 85 ff 75 e2 8b 40 10 85 f6 74 03 8b 76
All code
========
0: 89 c6 mov %eax,%esi
2: 53 push %rbx
3: 83 ec 08 sub $0x8,%esp
6: 89 55 ec mov %edx,-0x14(%rbp)
9: 8b 90 64 05 00 00 mov 0x564(%rax),%edx
f: 39 d1 cmp %edx,%ecx
11: 74 56 je 0x69
13: 8d 42 f8 lea -0x8(%rdx),%eax
16: eb 12 jmp 0x2a
18: 90 nop
19: 8b 5a 04 mov 0x4(%rdx),%ebx
1c: 85 db test %ebx,%ebx
1e: 74 17 je 0x37
20: 8b 50 08 mov 0x8(%rax),%edx
23: 8d 42 f8 lea -0x8(%rdx),%eax
26: 39 d1 cmp %edx,%ecx
28: 74 3f je 0x69
2a:* 8b 90 90 01 00 00 mov 0x190(%rax),%edx <-- trapping instruction
30: 8b 7a 08 mov 0x8(%rdx),%edi
33: 85 ff test %edi,%edi
35: 75 e2 jne 0x19
37: 8b 40 10 mov 0x10(%rax),%eax
3a: 85 f6 test %esi,%esi
3c: 74 03 je 0x41
3e: 8b .byte 0x8b
3f: 76 .byte 0x76
Code starting with the faulting instruction
===========================================
0: 8b 90 90 01 00 00 mov 0x190(%rax),%edx
6: 8b 7a 08 mov 0x8(%rdx),%edi
9: 85 ff test %edi,%edi
b: 75 e2 jne 0xffffffffffffffef
d: 8b 40 10 mov 0x10(%rax),%eax
10: 85 f6 test %esi,%esi
12: 74 03 je 0x17
14: 8b .byte 0x8b
15: 76 .byte 0x76
[ 4.729853][ T1] EAX: fffffff8 EBX: 86802000 ECX: 86802564 EDX: 00000000
[ 4.729853][ T1] ESI: 86802000 EDI: 86813400 EBP: 85e1fe90 ESP: 85e1fe7c
[ 4.729853][ T1] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS: 00010282
[ 4.729853][ T1] CR0: 80050033 CR2: 00000188 CR3: 05182000 CR4: 000406b0
[ 4.729853][ T1] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 4.729853][ T1] DR6: fffe0ff0 DR7: 00000400
[ 4.729853][ T1] Call Trace:
[ 4.729853][ T1] ? show_regs (arch/x86/kernel/dumpstack.c:478)
[ 4.729853][ T1] ? __die (arch/x86/kernel/dumpstack.c:421 arch/x86/kernel/dumpstack.c:434)
[ 4.729853][ T1] ? page_fault_oops (arch/x86/mm/fault.c:715)
[ 4.729853][ T1] ? kernelmode_fixup_or_oops+0x54/0x68
[ 4.729853][ T1] ? __bad_area_nosemaphore+0x103/0x180
[ 4.729853][ T1] ? sched_clock_noinstr (arch/x86/kernel/tsc.c:267)
[ 4.729853][ T1] ? bad_area_nosemaphore (arch/x86/mm/fault.c:835)
[ 4.729853][ T1] ? do_user_addr_fault (arch/x86/mm/fault.c:1452)
[ 4.729853][ T1] ? exc_page_fault (arch/x86/include/asm/irqflags.h:26 arch/x86/include/asm/irqflags.h:87 arch/x86/include/asm/irqflags.h:147 arch/x86/mm/fault.c:1489 arch/x86/mm/fault.c:1539)
[ 4.729853][ T1] ? pvclock_clocksource_read_nowd (arch/x86/mm/fault.c:1494)
[ 4.729853][ T1] ? handle_exception (arch/x86/entry/entry_32.S:1054)
[ 4.729853][ T1] ? pvclock_clocksource_read_nowd (arch/x86/mm/fault.c:1494)
[ 4.729853][ T1] ? drm_vblank_init (drivers/gpu/drm/drm_vblank.c:534)
[ 4.729853][ T1] ? pvclock_clocksource_read_nowd (arch/x86/mm/fault.c:1494)
[ 4.729853][ T1] ? drm_vblank_init (drivers/gpu/drm/drm_vblank.c:534)
[ 4.729853][ T1] vkms_create (drivers/gpu/drm/vkms/vkms_drv.c:211)
[ 4.729853][ T1] vkms_init (drivers/gpu/drm/vkms/vkms_drv.c:254)
[ 4.729853][ T1] ? vgem_init (drivers/gpu/drm/vkms/vkms_drv.c:240)
[ 4.729853][ T1] do_one_initcall (init/main.c:1267)
[ 4.729853][ T1] do_initcalls (init/main.c:1328 init/main.c:1345)
[ 4.729853][ T1] kernel_init_freeable (init/main.c:1580)
[ 4.729853][ T1] ? rest_init (init/main.c:1459)
[ 4.729853][ T1] kernel_init (init/main.c:1469)
[ 4.729853][ T1] ret_from_fork (arch/x86/kernel/process.c:153)
[ 4.729853][ T1] ? rest_init (init/main.c:1459)
[ 4.729853][ T1] ret_from_fork_asm (arch/x86/entry/entry_32.S:737)
[ 4.729853][ T1] entry_INT80_32 (arch/x86/entry/entry_32.S:944)
[ 4.729853][ T1] Modules linked in:
[ 4.729853][ T1] CR2: 0000000000000188
[ 4.729853][ T1] ---[ end trace 0000000000000000 ]---
[ 4.729853][ T1] EIP: drm_vblank_init (drivers/gpu/drm/drm_vblank.c:534)
[ 4.729853][ T1] Code: 89 c6 53 83 ec 08 89 55 ec 8b 90 64 05 00 00 39 d1 74 56 8d 42 f8 eb 12 90 8b 5a 04 85 db 74 17 8b 50 08 8d 42 f8 39 d1 74 3f <8b> 90 90 01 00 00 8b 7a 08 85 ff 75 e2 8b 40 10 85 f6 74 03 8b 76
All code
========
0: 89 c6 mov %eax,%esi
2: 53 push %rbx
3: 83 ec 08 sub $0x8,%esp
6: 89 55 ec mov %edx,-0x14(%rbp)
9: 8b 90 64 05 00 00 mov 0x564(%rax),%edx
f: 39 d1 cmp %edx,%ecx
11: 74 56 je 0x69
13: 8d 42 f8 lea -0x8(%rdx),%eax
16: eb 12 jmp 0x2a
18: 90 nop
19: 8b 5a 04 mov 0x4(%rdx),%ebx
1c: 85 db test %ebx,%ebx
1e: 74 17 je 0x37
20: 8b 50 08 mov 0x8(%rax),%edx
23: 8d 42 f8 lea -0x8(%rdx),%eax
26: 39 d1 cmp %edx,%ecx
28: 74 3f je 0x69
2a:* 8b 90 90 01 00 00 mov 0x190(%rax),%edx <-- trapping instruction
30: 8b 7a 08 mov 0x8(%rdx),%edi
33: 85 ff test %edi,%edi
35: 75 e2 jne 0x19
37: 8b 40 10 mov 0x10(%rax),%eax
3a: 85 f6 test %esi,%esi
3c: 74 03 je 0x41
3e: 8b .byte 0x8b
3f: 76 .byte 0x76
Code starting with the faulting instruction
===========================================
0: 8b 90 90 01 00 00 mov 0x190(%rax),%edx
6: 8b 7a 08 mov 0x8(%rdx),%edi
9: 85 ff test %edi,%edi
b: 75 e2 jne 0xffffffffffffffef
d: 8b 40 10 mov 0x10(%rax),%eax
10: 85 f6 test %esi,%esi
12: 74 03 je 0x17
14: 8b .byte 0x8b
15: 76 .byte 0x76
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20241010/202410101418.5704b4a5-lkp@intel.com
Le 27/09/24 - 16:39, Lyude Paul a écrit : > Currently, there's nothing actually stopping a driver from only registering > vblank support for some of it's CRTCs and not for others. As far as I can > tell, this isn't really defined behavior on the C side of things - as the > documentation explicitly mentions to not use drm_vblank_init() if you don't > have vblank support - since DRM then steps in and adds its own vblank > emulation implementation. > > So, let's fix this edge case and check to make sure it's all or none. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Fixes: 3ed4351a83ca ("drm: Extract drm_vblank.[hc]") > Cc: Stefan Agner <stefan@agner.ch> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@gmail.com> > Cc: Simona Vetter <simona@ffwll.ch> > Cc: dri-devel@lists.freedesktop.org > Cc: <stable@vger.kernel.org> # v4.13+ > --- > drivers/gpu/drm/drm_vblank.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 94e45ed6869d0..4d00937e8ca2e 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -525,9 +525,19 @@ static void drm_vblank_init_release(struct drm_device *dev, void *ptr) > */ > int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs) > { > + struct drm_crtc *crtc; > int ret; > unsigned int i; > > + // Confirm that the required vblank functions have been filled out for all CRTCS > + drm_for_each_crtc(crtc, dev) { > + if (!crtc->funcs->enable_vblank || !crtc->funcs->disable_vblank) { > + drm_err(dev, "CRTC vblank functions not initialized for %s, abort\n", > + crtc->name); > + return -EINVAL; > + } > + } > + Hi, I noticed that the kernel bot reported an issue with VKMS and this patch. I did not take the time to reproduce the issue, but it may come from the fact that VKMS call drm_vblank_init before calling drmm_crtc_init_with_planes [1]. I don't see anything in the documentation that requires the CRTC to be initialized before the vblank, is it a change of the API or an issue in VKMS since 2018 [2]? Anyway, if this is a requirement, can you explain it in the drm_vblank_init documentation? Thanks, Louis Chauvet [1]:https://elixir.bootlin.com/linux/v6.11.2/source/drivers/gpu/drm/vkms/vkms_drv.c#L209 [2]:https://lore.kernel.org/all/5d9ca7b3884c1995bd4a983b1d2ff1b840eb7f1a.1531402095.git.rodrigosiqueiramelo@gmail.com/ > spin_lock_init(&dev->vbl_lock); > spin_lock_init(&dev->vblank_time_lock); > > > base-commit: 22512c3ee0f47faab5def71c4453638923c62522 > -- > 2.46.1 >
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 94e45ed6869d0..4d00937e8ca2e 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -525,9 +525,19 @@ static void drm_vblank_init_release(struct drm_device *dev, void *ptr) */ int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs) { + struct drm_crtc *crtc; int ret; unsigned int i; + // Confirm that the required vblank functions have been filled out for all CRTCS + drm_for_each_crtc(crtc, dev) { + if (!crtc->funcs->enable_vblank || !crtc->funcs->disable_vblank) { + drm_err(dev, "CRTC vblank functions not initialized for %s, abort\n", + crtc->name); + return -EINVAL; + } + } + spin_lock_init(&dev->vbl_lock); spin_lock_init(&dev->vblank_time_lock);
Currently, there's nothing actually stopping a driver from only registering vblank support for some of it's CRTCs and not for others. As far as I can tell, this isn't really defined behavior on the C side of things - as the documentation explicitly mentions to not use drm_vblank_init() if you don't have vblank support - since DRM then steps in and adds its own vblank emulation implementation. So, let's fix this edge case and check to make sure it's all or none. Signed-off-by: Lyude Paul <lyude@redhat.com> Fixes: 3ed4351a83ca ("drm: Extract drm_vblank.[hc]") Cc: Stefan Agner <stefan@agner.ch> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: David Airlie <airlied@gmail.com> Cc: Simona Vetter <simona@ffwll.ch> Cc: dri-devel@lists.freedesktop.org Cc: <stable@vger.kernel.org> # v4.13+ --- drivers/gpu/drm/drm_vblank.c | 10 ++++++++++ 1 file changed, 10 insertions(+) base-commit: 22512c3ee0f47faab5def71c4453638923c62522