Linus, Eric,
On Mon, 8 Aug 2016 14:19:23 -0700
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, Aug 8, 2016 at 1:24 PM, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
> >
> > Booting up v4.8-rc1 in qemu today I ran I ran into this beautiful oops.
> >
> > I am just about to head out the door on vacation until the end of the
> > month so hopefully I am tossing out enough information to the right
> > people.
> >
> > I have confirmed that disabling CONFIG_DRM_CIRRUS_QEMU avoids this
> > crash.
> >
> > [ 0.720167] BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
> > [ 0.721348] IP: [<ffffffff8d5ef9ac>] drm_pick_crtcs+0xfc/0x270
> > [ 0.722249] PGD 0
> > [ 0.722659] Oops: 0000 [#1] SMP
> > [ 0.723141] Modules linked in:
> > [ 0.723643] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc1+ #116
> > [ 0.724602] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
> > [ 0.725450] task: ffff8800bbf28000 task.stack: ffff8800bbf30000
> > [ 0.726327] RIP: 0010:[<ffffffff8d5ef9ac>] [<ffffffff8d5ef9ac>] drm_pick_crtcs+0xfc/0x270
> > [ 0.727648] RSP: 0000:ffff8800bbf33978 EFLAGS: 00010217
> > [ 0.728444] RAX: ffffffff8d623a20 RBX: 0000000000000000 RCX: ffff8800baf3f860
> > [ 0.729498] RDX: 0000000000000000 RSI: 0000000000001000 RDI: ffff8800baf3f800
> > [ 0.730626] RBP: ffff8800bbf339f8 R08: 0000000000000000 R09: 0000000000000001
> > [ 0.731704] R10: 0000000000000001 R11: 0000000000000000 R12: ffff8800bbaf3af0
> > [ 0.732827] R13: ffff8800bbaf3af8 R14: 0000000000000000 R15: ffff8800baf3fc00
> > [ 0.733863] FS: 0000000000000000(0000) GS:ffff8800bf800000(0000) knlGS:0000000000000000
> > [ 0.735106] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 0.735964] CR2: 0000000000000018 CR3: 000000003b219000 CR4: 00000000000006f0
> > [ 0.737021] Stack:
> > [ 0.737342] ffff8800bbf33998 0000000000000246 ffff8800bbaf3b18 ffff8800bbaf3b00
> > [ 0.738602] 0000100000000001 0000000000000000 ffff8800bbaf3b00 ffff8800baf3f800
> > [ 0.739807] 0000000300001000 ffff8800bbaf3b18 ffff8800bbf339e8 ffff8800baf3fc00
> > [ 0.740993] Call Trace:
> > [ 0.741393] [<ffffffff8d5efedd>] drm_setup_crtcs+0x3bd/0xb50
> > [ 0.742252] [<ffffffff8d0edaa1>] ? mark_held_locks+0x71/0x90
> > [ 0.743176] [<ffffffff8db0db6b>] ? __mutex_unlock_slowpath+0xeb/0x1c0
> > [ 0.744138] [<ffffffff8d5f08c1>] drm_fb_helper_initial_config+0x81/0x3c0
> > [ 0.745166] [<ffffffff8d6110b4>] ? drm_modeset_unlock_all+0x54/0x80
> > [ 0.746136] [<ffffffff8d624fe0>] cirrus_fbdev_init+0xa0/0xb0
> > [ 0.747033] [<ffffffff8d6245fb>] cirrus_modeset_init+0x18b/0x1e0
> > .. snip snip ..
> > [ 0.771023] Code: e8 ba e1 ff ff 48 8b 55 b8 48 83 f8 01 83 5d c4 ff 48 8b 7d b8 48 8b 82 98 02 00 00 49 8b 57 08 48 8b 40 10 48 8b 92 00 0a 00 00 <48> 83 7a 18 00 74 09 48 85 c0 0f 84 53 01 00 00 ff d0 49 89 c3
>
> This decodes to
>
> 0: e8 ba e1 ff ff callq ...
> 5: 48 8b 55 b8 mov -0x48(%rbp),%rdx
> 9: 48 83 f8 01 cmp $0x1,%rax
> d: 83 5d c4 ff sbbl $0xffffffff,-0x3c(%rbp)
> 11: 48 8b 7d b8 mov -0x48(%rbp),%rdi
> 15: 48 8b 82 98 02 00 00 mov 0x298(%rdx),%rax
> 1c: 49 8b 57 08 mov 0x8(%r15),%rdx
> 20: 48 8b 40 10 mov 0x10(%rax),%rax
> 24: 48 8b 92 00 0a 00 00 mov 0xa00(%rdx),%rdx
> 2b:* 48 83 7a 18 00 cmpq $0x0,0x18(%rdx) <-- trapping instruction
> 30: 74 09 je 0x3b
> 32: 48 85 c0 test %rax,%rax
> 35: 0f 84 53 01 00 00 je 0x18e
> 3b: ff d0 callq *%rax
> 3d: 49 89 c3 mov %rax,%r11
>
>
> and trying to find matching code I think the oops is from this:
>
> if (fb_helper->dev->mode_config.funcs->atomic_commit &&
> !connector_funcs->best_encoder)
> encoder = drm_atomic_helper_best_encoder(connector);
> else
> encoder = connector_funcs->best_encoder(connector);
>
> in particular, the trapping instruction is the load of "atomic_commit".
>
> (The "callq *%rax" seems to be the "connector_funcs->best_encoder()" call)
>
> So I *think* we have "fb_helper->dev->mode_config.funcs" being NULL.
>
> But I might have gotten the code matching wrong. I don't have the same
> compiler as Eric, so even when using his config my code generation
> doesn't really match. But there is only one indirect call in that
> function, which is that ->best_encoder() call, so it does seem to
> match up.
>
> Adding Boris Brezillon and Daniel Vetter to the cc, since assuming I'm
> right, the main suspect is commit c61b93fe51b1 ("drm/atomic: Fix
> remaining places where !funcs->best_encoder is valid").
>
> Boris? Daniel?
I think you're right, c61b93fe51b1 is likely to be the commit who
introduced this bug. cirrus_driver_load() [1] assigns
mode_config->funcs pointer to &cirrus_mode_funcs only after calling
cirrus_modeset_init() (this function is taking care of initializing all
DRM elements including the fbdev emulation layer), and commit
c61b93fe51b1 introduced a test on mode_config->funcs->atomic_commit in
the fbdev emulation init code, hence the crash.
Re-ordering the init steps like this [2] should fix the problem (Eric,
can you give this patch a try).
Regards,
Boris
[1]http://lxr.free-electrons.com/source/drivers/gpu/drm/cirrus/cirrus_main.c#L166
[2]http://code.bulix.org/jcs6d9-104989
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
