From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: Remove bogus __init annotation from DMI callbacks Date: Thu, 28 Aug 2014 10:10:01 +0300 Message-ID: <87zjephyye.fsf@intel.com> References: <1409157679-20111-1-git-send-email-minipli@googlemail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1409157679-20111-1-git-send-email-minipli@googlemail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, Rusty Russell , dri-devel@lists.freedesktop.org, Jarod Wilson , stable@vger.kernel.org, Mathias Krause , Duncan Laurie List-Id: intel-gfx@lists.freedesktop.org On Wed, 27 Aug 2014, Mathias Krause 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 > Cc: Daniel Vetter > Cc: Duncan Laurie > Cc: Jarod Wilson > Cc: Rusty Russell # 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