Re: [PATCH] drm/i915: Remove bogus __init annotation from DMI callbacks

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 27 Aug 2014, Mathias Krause <minipli@googlemail.com> wrote:
> The __init annotations for the DMI callback functions are wrong as this
> code can be called even after the module has been initialized, e.g. like
> this:
>
>   # echo 1 > /sys/bus/pci/devices/0000:00:02.0/remove
>   # modprobe i915
>   # echo 1 > /sys/bus/pci/rescan
>
> The first command will remove the PCI device from the kernel's device
> list so the second command won't see it right away. But as it registers
> a PCI driver it'll see it on the third command. If the system happens to
> match one of the DMI table entries we'll try to call a function in long
> released memory and generate an Oops, at best.
>
> Fix this by removing the bogus annotation.
>
> Modpost should have caught that one but it ignores section reference
> mismatches from the .rodata section. :/
>
> Fixes: 25e341cfc33d ("drm/i915: quirk away broken OpRegion VBT")
> Fixes: 8ca4013d702d ("CHROMIUM: i915: Add DMI override to skip CRT...")
> Fixes: 425d244c8670 ("drm/i915: ignore LVDS on intel graphics systems...")
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Duncan Laurie <dlaurie@chromium.org>
> Cc: Jarod Wilson <jarod@redhat.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>	# Can modpost be fixed?
> Cc: stable@vger.kernel.org

Nice catch! Thanks for the patch, pushed to drm-intel-fixes.

BR,
Jani.



> ---
>
> In the long run me might want to move the DMI tests to some __init code
> to be able to mark the DMI tables as __initconst, thereby allowing to
> release this memory after module initialization. That would safe us some
> ~11 kB of memory, as the DMI data shouldn't change at run-time.
>
>  drivers/gpu/drm/i915/intel_bios.c |    2 +-
>  drivers/gpu/drm/i915/intel_crt.c  |    2 +-
>  drivers/gpu/drm/i915/intel_lvds.c |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index a66955037e4e..eee79e1c3222 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1123,7 +1123,7 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> -static int __init intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
> +static int intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
>  {
>  	DRM_DEBUG_KMS("Falling back to manually reading VBT from "
>  		      "VBIOS ROM for %s\n",
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index e8abfce40976..9212e6504e0f 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -804,7 +804,7 @@ static const struct drm_encoder_funcs intel_crt_enc_funcs = {
>  	.destroy = intel_encoder_destroy,
>  };
>  
> -static int __init intel_no_crt_dmi_callback(const struct dmi_system_id *id)
> +static int intel_no_crt_dmi_callback(const struct dmi_system_id *id)
>  {
>  	DRM_INFO("Skipping CRT initialization for %s\n", id->ident);
>  	return 1;
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 881361c0f27e..fdf40267249c 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -538,7 +538,7 @@ static const struct drm_encoder_funcs intel_lvds_enc_funcs = {
>  	.destroy = intel_encoder_destroy,
>  };
>  
> -static int __init intel_no_lvds_dmi_callback(const struct dmi_system_id *id)
> +static int intel_no_lvds_dmi_callback(const struct dmi_system_id *id)
>  {
>  	DRM_INFO("Skipping LVDS initialization for %s\n", id->ident);
>  	return 1;
> -- 
> 1.7.10.4
>

-- 
Jani Nikula, Intel Open Source Technology Center