linux-stable/include/drm/drm_panic.h
Lyude Paul 319e53f155 drm/panic: Fix uninitialized spinlock acquisition with CONFIG_DRM_PANIC=n
It turns out that if you happen to have a kernel config where
CONFIG_DRM_PANIC is disabled and spinlock debugging is enabled, along with
KMS being enabled - we'll end up trying to acquire an uninitialized
spin_lock with drm_panic_lock() when we try to do a commit:

  rvkms rvkms.0: [drm:drm_atomic_commit] committing 0000000068d2ade1
  INFO: trying to register non-static key.
  The code is fine but needs lockdep annotation, or maybe
  you didn't initialize this object before use?
  turning off the locking correctness validator.
  CPU: 4 PID: 1347 Comm: modprobe Not tainted 6.10.0-rc1Lyude-Test+ #272
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS edk2-20240524-3.fc40 05/24/2024
  Call Trace:
   <TASK>
   dump_stack_lvl+0x77/0xa0
   assign_lock_key+0x114/0x120
   register_lock_class+0xa8/0x2c0
   __lock_acquire+0x7d/0x2bd0
   ? __vmap_pages_range_noflush+0x3a8/0x550
   ? drm_atomic_helper_swap_state+0x2ad/0x3a0
   lock_acquire+0xec/0x290
   ? drm_atomic_helper_swap_state+0x2ad/0x3a0
   ? lock_release+0xee/0x310
   _raw_spin_lock_irqsave+0x4e/0x70
   ? drm_atomic_helper_swap_state+0x2ad/0x3a0
   drm_atomic_helper_swap_state+0x2ad/0x3a0
   drm_atomic_helper_commit+0xb1/0x270
   drm_atomic_commit+0xaf/0xe0
   ? __pfx___drm_printfn_info+0x10/0x10
   drm_client_modeset_commit_atomic+0x1a1/0x250
   drm_client_modeset_commit_locked+0x4b/0x180
   drm_client_modeset_commit+0x27/0x50
   __drm_fb_helper_restore_fbdev_mode_unlocked+0x76/0x90
   drm_fb_helper_set_par+0x38/0x40
   fbcon_init+0x3c4/0x690
   visual_init+0xc0/0x120
   do_bind_con_driver+0x409/0x4c0
   do_take_over_console+0x233/0x280
   do_fb_registered+0x11f/0x210
   fbcon_fb_registered+0x2c/0x60
   register_framebuffer+0x248/0x2a0
   __drm_fb_helper_initial_config_and_unlock+0x58a/0x720
   drm_fbdev_generic_client_hotplug+0x6e/0xb0
   drm_client_register+0x76/0xc0
   _RNvXs_CsHeezP08sTT_5rvkmsNtB4_5RvkmsNtNtCs1cdwasc6FUb_6kernel8platform6Driver5probe+0xed2/0x1060 [rvkms]
   ? _RNvMs_NtCs1cdwasc6FUb_6kernel8platformINtB4_7AdapterNtCsHeezP08sTT_5rvkms5RvkmsE14probe_callbackBQ_+0x2b/0x70 [rvkms]
   ? acpi_dev_pm_attach+0x25/0x110
   ? platform_probe+0x6a/0xa0
   ? really_probe+0x10b/0x400
   ? __driver_probe_device+0x7c/0x140
   ? driver_probe_device+0x22/0x1b0
   ? __device_attach_driver+0x13a/0x1c0
   ? __pfx___device_attach_driver+0x10/0x10
   ? bus_for_each_drv+0x114/0x170
   ? __device_attach+0xd6/0x1b0
   ? bus_probe_device+0x9e/0x120
   ? device_add+0x288/0x4b0
   ? platform_device_add+0x75/0x230
   ? platform_device_register_full+0x141/0x180
   ? rust_helper_platform_device_register_simple+0x85/0xb0
   ? _RNvMs2_NtCs1cdwasc6FUb_6kernel8platformNtB5_6Device13create_simple+0x1d/0x60
   ? _RNvXs0_CsHeezP08sTT_5rvkmsNtB5_5RvkmsNtCs1cdwasc6FUb_6kernel6Module4init+0x11e/0x160 [rvkms]
   ? 0xffffffffc083f000
   ? init_module+0x20/0x1000 [rvkms]
   ? kernfs_xattr_get+0x3e/0x80
   ? do_one_initcall+0x148/0x3f0
   ? __lock_acquire+0x5ef/0x2bd0
   ? __lock_acquire+0x5ef/0x2bd0
   ? __lock_acquire+0x5ef/0x2bd0
   ? put_cpu_partial+0x51/0x1d0
   ? lock_acquire+0xec/0x290
   ? put_cpu_partial+0x51/0x1d0
   ? lock_release+0xee/0x310
   ? put_cpu_partial+0x51/0x1d0
   ? fs_reclaim_acquire+0x69/0xf0
   ? lock_acquire+0xec/0x290
   ? fs_reclaim_acquire+0x69/0xf0
   ? kfree+0x22f/0x340
   ? lock_release+0xee/0x310
   ? kmalloc_trace_noprof+0x48/0x340
   ? do_init_module+0x22/0x240
   ? kmalloc_trace_noprof+0x155/0x340
   ? do_init_module+0x60/0x240
   ? __se_sys_finit_module+0x2e0/0x3f0
   ? do_syscall_64+0xa4/0x180
   ? syscall_exit_to_user_mode+0x108/0x140
   ? do_syscall_64+0xb0/0x180
   ? vma_end_read+0xd0/0xe0
   ? do_user_addr_fault+0x309/0x640
   ? clear_bhb_loop+0x45/0xa0
   ? clear_bhb_loop+0x45/0xa0
   ? clear_bhb_loop+0x45/0xa0
   ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
   </TASK>

Fix this by stubbing these macros out when this config option isn't
enabled, along with fixing the unused variable warning that introduces.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Fixes: e2a1cda3e0 ("drm/panic: Add drm panic locking")
Cc: <stable@vger.kernel.org> # v6.10+
Link: https://patchwork.freedesktop.org/patch/msgid/20240916230103.611490-1-lyude@redhat.com
2024-09-16 19:03:29 -04:00

167 lines
5.3 KiB
C

/* SPDX-License-Identifier: GPL-2.0 or MIT */
/*
* Copyright (c) 2024 Intel
* Copyright (c) 2024 Red Hat
*/
#ifndef __DRM_PANIC_H__
#define __DRM_PANIC_H__
#include <linux/module.h>
#include <linux/types.h>
#include <linux/iosys-map.h>
#include <drm/drm_device.h>
#include <drm/drm_fourcc.h>
/**
* struct drm_scanout_buffer - DRM scanout buffer
*
* This structure holds the information necessary for drm_panic to draw the
* panic screen, and display it.
*/
struct drm_scanout_buffer {
/**
* @format:
*
* drm format of the scanout buffer.
*/
const struct drm_format_info *format;
/**
* @map:
*
* Virtual address of the scanout buffer, either in memory or iomem.
* The scanout buffer should be in linear format, and can be directly
* sent to the display hardware. Tearing is not an issue for the panic
* screen.
*/
struct iosys_map map[DRM_FORMAT_MAX_PLANES];
/**
* @width: Width of the scanout buffer, in pixels.
*/
unsigned int width;
/**
* @height: Height of the scanout buffer, in pixels.
*/
unsigned int height;
/**
* @pitch: Length in bytes between the start of two consecutive lines.
*/
unsigned int pitch[DRM_FORMAT_MAX_PLANES];
/**
* @set_pixel: Optional function, to set a pixel color on the
* framebuffer. It allows to handle special tiling format inside the
* driver.
*/
void (*set_pixel)(struct drm_scanout_buffer *sb, unsigned int x,
unsigned int y, u32 color);
};
#ifdef CONFIG_DRM_PANIC
/**
* drm_panic_trylock - try to enter the panic printing critical section
* @dev: struct drm_device
* @flags: unsigned long irq flags you need to pass to the unlock() counterpart
*
* This function must be called by any panic printing code. The panic printing
* attempt must be aborted if the trylock fails.
*
* Panic printing code can make the following assumptions while holding the
* panic lock:
*
* - Anything protected by drm_panic_lock() and drm_panic_unlock() pairs is safe
* to access.
*
* - Furthermore the panic printing code only registers in drm_dev_unregister()
* and gets removed in drm_dev_unregister(). This allows the panic code to
* safely access any state which is invariant in between these two function
* calls, like the list of planes &drm_mode_config.plane_list or most of the
* struct drm_plane structure.
*
* Specifically thanks to the protection around plane updates in
* drm_atomic_helper_swap_state() the following additional guarantees hold:
*
* - It is safe to deference the drm_plane.state pointer.
*
* - Anything in struct drm_plane_state or the driver's subclass thereof which
* stays invariant after the atomic check code has finished is safe to access.
* Specifically this includes the reference counted pointers to framebuffer
* and buffer objects.
*
* - Anything set up by &drm_plane_helper_funcs.fb_prepare and cleaned up
* &drm_plane_helper_funcs.fb_cleanup is safe to access, as long as it stays
* invariant between these two calls. This also means that for drivers using
* dynamic buffer management the framebuffer is pinned, and therefer all
* relevant datastructures can be accessed without taking any further locks
* (which would be impossible in panic context anyway).
*
* - Importantly, software and hardware state set up by
* &drm_plane_helper_funcs.begin_fb_access and
* &drm_plane_helper_funcs.end_fb_access is not safe to access.
*
* Drivers must not make any assumptions about the actual state of the hardware,
* unless they explicitly protected these hardware access with drm_panic_lock()
* and drm_panic_unlock().
*
* Return:
* %0 when failing to acquire the raw spinlock, nonzero on success.
*/
#define drm_panic_trylock(dev, flags) \
raw_spin_trylock_irqsave(&(dev)->mode_config.panic_lock, flags)
/**
* drm_panic_lock - protect panic printing relevant state
* @dev: struct drm_device
* @flags: unsigned long irq flags you need to pass to the unlock() counterpart
*
* This function must be called to protect software and hardware state that the
* panic printing code must be able to rely on. The protected sections must be
* as small as possible. It uses the irqsave/irqrestore variant, and can be
* called from irq handler. Examples include:
*
* - Access to peek/poke or other similar registers, if that is the way the
* driver prints the pixels into the scanout buffer at panic time.
*
* - Updates to pointers like &drm_plane.state, allowing the panic handler to
* safely deference these. This is done in drm_atomic_helper_swap_state().
*
* - An state that isn't invariant and that the driver must be able to access
* during panic printing.
*/
#define drm_panic_lock(dev, flags) \
raw_spin_lock_irqsave(&(dev)->mode_config.panic_lock, flags)
/**
* drm_panic_unlock - end of the panic printing critical section
* @dev: struct drm_device
* @flags: irq flags that were returned when acquiring the lock
*
* Unlocks the raw spinlock acquired by either drm_panic_lock() or
* drm_panic_trylock().
*/
#define drm_panic_unlock(dev, flags) \
raw_spin_unlock_irqrestore(&(dev)->mode_config.panic_lock, flags)
#else
static inline bool drm_panic_trylock(struct drm_device *dev, unsigned long flags)
{
return true;
}
static inline void drm_panic_lock(struct drm_device *dev, unsigned long flags) {}
static inline void drm_panic_unlock(struct drm_device *dev, unsigned long flags) {}
#endif
#endif /* __DRM_PANIC_H__ */