* [PATCH] drm/vblank: Require a driver register vblank support for 0 or all CRTCs
@ 2024-09-27 20:39 Lyude Paul
2024-10-10 6:49 ` kernel test robot
2024-10-10 15:26 ` Louis Chauvet
0 siblings, 2 replies; 3+ messages in thread
From: Lyude Paul @ 2024-09-27 20:39 UTC (permalink / raw)
To: dri-devel
Cc: Stefan Agner, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, stable, open list
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;
+ }
+ }
+
spin_lock_init(&dev->vbl_lock);
spin_lock_init(&dev->vblank_time_lock);
base-commit: 22512c3ee0f47faab5def71c4453638923c62522
--
2.46.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/vblank: Require a driver register vblank support for 0 or all CRTCs
2024-09-27 20:39 [PATCH] drm/vblank: Require a driver register vblank support for 0 or all CRTCs Lyude Paul
@ 2024-10-10 6:49 ` kernel test robot
2024-10-10 15:26 ` Louis Chauvet
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2024-10-10 6:49 UTC (permalink / raw)
To: Lyude Paul
Cc: oe-lkp, lkp, Stefan Agner, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
dri-devel, stable, oliver.sang
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/vblank: Require a driver register vblank support for 0 or all CRTCs
2024-09-27 20:39 [PATCH] drm/vblank: Require a driver register vblank support for 0 or all CRTCs Lyude Paul
2024-10-10 6:49 ` kernel test robot
@ 2024-10-10 15:26 ` Louis Chauvet
1 sibling, 0 replies; 3+ messages in thread
From: Louis Chauvet @ 2024-10-10 15:26 UTC (permalink / raw)
To: Lyude Paul
Cc: dri-devel, Stefan Agner, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
stable, open list
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
>
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-10-10 15:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-27 20:39 [PATCH] drm/vblank: Require a driver register vblank support for 0 or all CRTCs Lyude Paul
2024-10-10 6:49 ` kernel test robot
2024-10-10 15:26 ` Louis Chauvet
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.