linux-kernel - Re: [-mm patch] drivers/pci/quirks.c: cleanup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070107154441.GB22558@jupiter.solarsys.private>
Date:	Sun, 7 Jan 2007 10:44:41 -0500
From:	"Mark M. Hoffman" <mhoffman@...htlink.com>
To:	Jean Delvare <khali@...ux-fr.org>
Cc:	Adrian Bunk <bunk@...sta.de>, greg@...ah.com,
	linux-pci@...ey.karlin.mff.cuni.cz, linux-kernel@...r.kernel.org
Subject: Re: [-mm patch] drivers/pci/quirks.c: cleanup

Hi Jean, Adrian:

> On Tue, 19 Dec 2006 05:13:15 +0100, Adrian Bunk wrote:
> > @@ -1122,6 +1123,14 @@ static void quirk_sis_96x_smbus(struct p
> >  	pci_write_config_byte(dev, 0x77, val & ~0x10);
> >  	pci_read_config_byte(dev, 0x77, &val);
> >  }
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI,	PCI_DEVICE_ID_SI_961,		quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI,	PCI_DEVICE_ID_SI_962,		quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI,	PCI_DEVICE_ID_SI_963,		quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI,	PCI_DEVICE_ID_SI_LPC,		quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI,	PCI_DEVICE_ID_SI_961,		quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI,	PCI_DEVICE_ID_SI_962,		quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI,	PCI_DEVICE_ID_SI_963,		quirk_sis_96x_smbus );
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI,	PCI_DEVICE_ID_SI_LPC,		quirk_sis_96x_smbus );
> >  
> >  /*
> >   * ... This is further complicated by the fact that some SiS96x south
> > @@ -1158,6 +1167,8 @@ static void quirk_sis_503(struct pci_dev
> >  	 */
> >  	dev->device = devid;
> >  }
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SI,	PCI_DEVICE_ID_SI_503,		quirk_sis_503 );
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_SI,	PCI_DEVICE_ID_SI_503,		quirk_sis_503 );

* Jean Delvare <khali@...ux-fr.org> [2007-01-05 09:52:33 +0100]:
> Was this patch tested on the SiS-based boards which need these quirks?
> I think you broke them. If I remember correctly, quirk_sis_503() must
> be called before quirk_sis_96x_smbus() for some boards to work
> properly, and we currently rely on the linking order to guarantee that.
> Likewise, quirk_sis_96x_compatible() should be called before
> quirk_sis_503() otherwise the warning message in quirk_sis_503() will
> no longer be correct.
> 
> So if you want to put the calls right after the quirk functions, you
> need to reorder the functions themselves as well. Feel free to add a
> comment explaining the order requirement so that nobody breaks it
> accidentally again in the future.

It is fragile for this code to depend on link order; Adrian's obvious and
trivial cleanups broke it.  Not only that, but some FC kernels had/have the
link order reversed such that this quirk is broken anyway.

I sent a patch for this back in May:
http://lists.lm-sensors.org/pipermail/lm-sensors/2006-May/016113.html

There was some discussion on the linux-pci mailing list as well; can't seem to
find an archive of that though.  Basically, it was not understood how the FC
kernels could have a reversed link order.  I never followed up on it, my bad.

At any rate, can we please get the patch above applied?  I will send a new one
if necessary.

Regards,

-- 
Mark M. Hoffman
mhoffman@...htlink.com

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/