* Re: [lm-sensors] [PATCH] i2c-piix4: Blacklist two mainboards
2008-05-04 16:30 [lm-sensors] [PATCH] i2c-piix4: Blacklist two mainboards Jean Delvare
@ 2008-05-12 12:17 ` Jean Delvare
2008-05-12 13:32 ` Achim Gottinger
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2008-05-12 12:17 UTC (permalink / raw)
To: lm-sensors
On Sun, 4 May 2008 18:30:48 +0200, Jean Delvare wrote:
> We had a report that running sensors-detect on a Sapphire AM2RD790
> motherbord killed the CPU. While the exact cause is still unknown,
> I'd rather play it safe and prevent any access to the SMBus on that
> machine by not letting the i2c-piix4 driver attach to the SMBus host
> device on that machine. Also blacklist a similar board made by DFI.
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> ---
> Previous version was broken, sorry.
>
> drivers/i2c/busses/i2c-piix4.c | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
> --- linux-2.6.25.orig/drivers/i2c/busses/i2c-piix4.c 2008-04-19 11:11:56.000000000 +0200
> +++ linux-2.6.25/drivers/i2c/busses/i2c-piix4.c 2008-05-04 09:56:00.000000000 +0200
> @@ -108,7 +108,27 @@ static unsigned short piix4_smba;
> static struct pci_driver piix4_driver;
> static struct i2c_adapter piix4_adapter;
>
> -static struct dmi_system_id __devinitdata piix4_dmi_table[] = {
> +static struct dmi_system_id __devinitdata piix4_dmi_blacklist[] = {
> + {
> + .ident = "Sapphire AM2RD790",
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "SAPPHIRE Inc."),
> + DMI_MATCH(DMI_BOARD_NAME, "PC-AM2RD790"),
> + },
> + },
> + {
> + .ident = "DFI Lanparty UT 790FX",
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "DFI Inc."),
> + DMI_MATCH(DMI_BOARD_NAME, "LP UT 790FX"),
> + },
> + },
> + { }
> +};
> +
> +/* The IBM entry is in a separate table because we only check it
> + on Intel-based systems */
> +static struct dmi_system_id __devinitdata piix4_dmi_ibm[] = {
> {
> .ident = "IBM",
> .matches = { DMI_MATCH(DMI_SYS_VENDOR, "IBM"), },
> @@ -123,8 +143,16 @@ static int __devinit piix4_setup(struct
>
> dev_info(&PIIX4_dev->dev, "Found %s device\n", pci_name(PIIX4_dev));
>
> + /* On some motherboards, it was reported that accessing the SMBus
> + caused severe hardware problems */
> + if (dmi_check_system(piix4_dmi_blacklist)) {
> + dev_err(&PIIX4_dev->dev,
> + "Accessing the SMBus on this system is unsafe!\n");
> + return -EPERM;
> + }
> +
> /* Don't access SMBus on IBM systems which get corrupted eeproms */
> - if (dmi_check_system(piix4_dmi_table) &&
> + if (dmi_check_system(piix4_dmi_ibm) &&
> PIIX4_dev->vendor = PCI_VENDOR_ID_INTEL) {
> dev_err(&PIIX4_dev->dev, "IBM system detected; this module "
> "may corrupt your serial eeprom! Refusing to load "
>
Achim, did you have a chance to give a try to this patch? I understand
that you don't want to apply it permanently while you're still having
fun with the I2C devices on the SMBus, but if you could still test it
and report whether it works as intended, that would be great. This
patch is in kernel 2.6.26-rc2 now, I would like to backport it to
2.6.25.y, but before I do I'd like to have at least one positive test
result.
Thanks,
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [lm-sensors] [PATCH] i2c-piix4: Blacklist two mainboards
2008-05-04 16:30 [lm-sensors] [PATCH] i2c-piix4: Blacklist two mainboards Jean Delvare
2008-05-12 12:17 ` Jean Delvare
@ 2008-05-12 13:32 ` Achim Gottinger
2008-05-12 14:19 ` Jean Delvare
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Achim Gottinger @ 2008-05-12 13:32 UTC (permalink / raw)
To: lm-sensors
Jean Delvare schrieb:
> On Sun, 4 May 2008 18:30:48 +0200, Jean Delvare wrote:
>
>> We had a report that running sensors-detect on a Sapphire AM2RD790
>> motherbord killed the CPU. While the exact cause is still unknown,
>> I'd rather play it safe and prevent any access to the SMBus on that
>> machine by not letting the i2c-piix4 driver attach to the SMBus host
>> device on that machine. Also blacklist a similar board made by DFI.
>>
>> Signed-off-by: Jean Delvare <khali@linux-fr.org>
>> ---
>> Previous version was broken, sorry.
>>
>> drivers/i2c/busses/i2c-piix4.c | 32 ++++++++++++++++++++++++++++++--
>> 1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> --- linux-2.6.25.orig/drivers/i2c/busses/i2c-piix4.c 2008-04-19 11:11:56.000000000 +0200
>> +++ linux-2.6.25/drivers/i2c/busses/i2c-piix4.c 2008-05-04 09:56:00.000000000 +0200
>> @@ -108,7 +108,27 @@ static unsigned short piix4_smba;
>> static struct pci_driver piix4_driver;
>> static struct i2c_adapter piix4_adapter;
>>
>> -static struct dmi_system_id __devinitdata piix4_dmi_table[] = {
>> +static struct dmi_system_id __devinitdata piix4_dmi_blacklist[] = {
>> + {
>> + .ident = "Sapphire AM2RD790",
>> + .matches = {
>> + DMI_MATCH(DMI_BOARD_VENDOR, "SAPPHIRE Inc."),
>> + DMI_MATCH(DMI_BOARD_NAME, "PC-AM2RD790"),
>> + },
>> + },
>> + {
>> + .ident = "DFI Lanparty UT 790FX",
>> + .matches = {
>> + DMI_MATCH(DMI_BOARD_VENDOR, "DFI Inc."),
>> + DMI_MATCH(DMI_BOARD_NAME, "LP UT 790FX"),
>> + },
>> + },
>> + { }
>> +};
>> +
>> +/* The IBM entry is in a separate table because we only check it
>> + on Intel-based systems */
>> +static struct dmi_system_id __devinitdata piix4_dmi_ibm[] = {
>> {
>> .ident = "IBM",
>> .matches = { DMI_MATCH(DMI_SYS_VENDOR, "IBM"), },
>> @@ -123,8 +143,16 @@ static int __devinit piix4_setup(struct
>>
>> dev_info(&PIIX4_dev->dev, "Found %s device\n", pci_name(PIIX4_dev));
>>
>> + /* On some motherboards, it was reported that accessing the SMBus
>> + caused severe hardware problems */
>> + if (dmi_check_system(piix4_dmi_blacklist)) {
>> + dev_err(&PIIX4_dev->dev,
>> + "Accessing the SMBus on this system is unsafe!\n");
>> + return -EPERM;
>> + }
>> +
>> /* Don't access SMBus on IBM systems which get corrupted eeproms */
>> - if (dmi_check_system(piix4_dmi_table) &&
>> + if (dmi_check_system(piix4_dmi_ibm) &&
>> PIIX4_dev->vendor = PCI_VENDOR_ID_INTEL) {
>> dev_err(&PIIX4_dev->dev, "IBM system detected; this module "
>> "may corrupt your serial eeprom! Refusing to load "
>>
>>
>
> Achim, did you have a chance to give a try to this patch? I understand
> that you don't want to apply it permanently while you're still having
> fun with the I2C devices on the SMBus, but if you could still test it
> and report whether it works as intended, that would be great. This
> patch is in kernel 2.6.26-rc2 now, I would like to backport it to
> 2.6.25.y, but before I do I'd like to have at least one positive test
> result.
>
> Thanks,
>
Test positive:
piix4_smbus 0000:00:14.0: Found 0000:00:14.0 device
piix4_smbus 0000:00:14.0: Accessing the SMBus on this system is unsafe!
piix4_smbus: probe of 0000:00:14.0 failed with error -1
But the i2c-piix4 module never made a problem it was only sensors-detect.
About that overclocking module i'm working on. I moved all my kernel
stuff into a new dir called hwmod and made a modified hwmod.c from
hwmon.c. Made all the required modifications
on Kconfig and Makefile and stuff works fine. Devices use their own
class /sysfs/class/hwmod.
I also added a new adapter class I2C_CLASS_HWMOD = (1<<7). To use the
i2c-piix4 module i had to add that class id to the i2c_adapter struct in
there.
I see no requirement to write separate modules for the smbus
controllers. I guess there is way to use I2C_CLASS_HWMON adapters for
devices ending up in /sysfs/class/hwmod, till i figured out how i'll
simply patch i2c-piix4.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [lm-sensors] [PATCH] i2c-piix4: Blacklist two mainboards
2008-05-04 16:30 [lm-sensors] [PATCH] i2c-piix4: Blacklist two mainboards Jean Delvare
2008-05-12 12:17 ` Jean Delvare
2008-05-12 13:32 ` Achim Gottinger
@ 2008-05-12 14:19 ` Jean Delvare
2008-05-12 15:02 ` Achim Gottinger
2008-05-12 15:26 ` Jean Delvare
4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2008-05-12 14:19 UTC (permalink / raw)
To: lm-sensors
On Mon, 12 May 2008 15:32:26 +0200, Achim Gottinger wrote:
> Jean Delvare schrieb:
> > Achim, did you have a chance to give a try to this patch? I understand
> > that you don't want to apply it permanently while you're still having
> > fun with the I2C devices on the SMBus, but if you could still test it
> > and report whether it works as intended, that would be great. This
> > patch is in kernel 2.6.26-rc2 now, I would like to backport it to
> > 2.6.25.y, but before I do I'd like to have at least one positive test
> > result.
>
> Test positive:
>
> piix4_smbus 0000:00:14.0: Found 0000:00:14.0 device
> piix4_smbus 0000:00:14.0: Accessing the SMBus on this system is unsafe!
> piix4_smbus: probe of 0000:00:14.0 failed with error -1
OK, thank you. I'll push the patch to 2.6.25.y now.
The error message might frighten people though, maybe we'll have to
change the error code so that no message shows in the future.
> But the i2c-piix4 module never made a problem it was only sensors-detect.
This is how you hit the problem, but there are many other ways people
could hit it, such as using i2cdump, i2cget or i2cset, or even just
loading a hardware monitoring driver which happens to probe I2C address
0x2e. While we fixed sensors-detect by now, not everyone will upgrade
to the new version of the script immediately, and all the other ways to
screw it up are still available. So, the only safe thing to do at the
moment is to disable the SMBus on your motherboard completely.
In the future, we could refine this and allow access again with
restrictions, such as no probing allowed from kernel drivers, and no
access from user-space (there's some more work needed before we can do
that.) Or we could limit the blacklisting to specific addresses (that's
easier to implement).
> About that overclocking module i'm working on. I moved all my kernel
> stuff into a new dir called hwmod and made a modified hwmod.c from
> hwmon.c. Made all the required modifications
> on Kconfig and Makefile and stuff works fine. Devices use their own
> class /sysfs/class/hwmod.
> I also added a new adapter class I2C_CLASS_HWMOD = (1<<7). To use the
> i2c-piix4 module i had to add that class id to the i2c_adapter struct in
> there.
I am worried that "hwmod" is too similar to "hwmon" and this will lead
to confusion. What about "overclock" or a similarly explicit term?
> I see no requirement to write separate modules for the smbus
> controllers. I guess there is way to use I2C_CLASS_HWMON adapters for
> devices ending up in /sysfs/class/hwmod, till i figured out how i'll
> simply patch i2c-piix4.
There is indeed no reason for separate modules for the SMBus
controllers, I'm not even sure how you came to think we could do that.
Adding a new class flag to i2c-piix4 was the right thing to do.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [lm-sensors] [PATCH] i2c-piix4: Blacklist two mainboards
2008-05-04 16:30 [lm-sensors] [PATCH] i2c-piix4: Blacklist two mainboards Jean Delvare
` (2 preceding siblings ...)
2008-05-12 14:19 ` Jean Delvare
@ 2008-05-12 15:02 ` Achim Gottinger
2008-05-12 15:26 ` Jean Delvare
4 siblings, 0 replies; 6+ messages in thread
From: Achim Gottinger @ 2008-05-12 15:02 UTC (permalink / raw)
To: lm-sensors
> This is how you hit the problem, but there are many other ways people
> could hit it, such as using i2cdump, i2cget or i2cset, or even just
> loading a hardware monitoring driver which happens to probe I2C address
> 0x2e. While we fixed sensors-detect by now, not everyone will upgrade
> to the new version of the script immediately, and all the other ways to
> screw it up are still available. So, the only safe thing to do at the
> moment is to disable the SMBus on your motherboard completely.
>
> In the future, we could refine this and allow access again with
> restrictions, such as no probing allowed from kernel drivers, and no
> access from user-space (there's some more work needed before we can do
> that.) Or we could limit the blacklisting to specific addresses (that's
> easier to implement).
>
>
But that way you can not run it on the DFI and Sapphire boards even if
the latest lm-sensors version is
in use. How about a kernel config flag to disable this patch? Also I
noted that you added infos on the website about this board.
The DFI Lanparty UT 790FX-M2R is not mentioned there but this board is
also affected, two users reported they had to rma
board and/or cpu after running sensors-detect meanwile.
> I am worried that "hwmod" is too similar to "hwmon" and this will lead
> to confusion. What about "overclock" or a similarly explicit term?
>
>
I'm concerned about that similarity here also. I don't want to put the
focus on overclocking, so
i picked hwmod (hardware modification) for now, can use hwchange,hwshift
or hwco (hardware change/clock over)
also. It's easy to change this in future.
>> I see no requirement to write separate modules for the smbus
>> controllers. I guess there is way to use I2C_CLASS_HWMON adapters for
>> devices ending up in /sysfs/class/hwmod, till i figured out how i'll
>> simply patch i2c-piix4.
>>
>
> There is indeed no reason for separate modules for the SMBus
> controllers, I'm not even sure how you came to think we could do that.
> Adding a new class flag to i2c-piix4 was the right thing to do.
Good, that keeps things simple. I did not expect to run a hwmod version
of that module parallel to the hwmon version.
Is this list the right place to discuss i2c related kernel modules? I
dont want to get off topic here.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [lm-sensors] [PATCH] i2c-piix4: Blacklist two mainboards
2008-05-04 16:30 [lm-sensors] [PATCH] i2c-piix4: Blacklist two mainboards Jean Delvare
` (3 preceding siblings ...)
2008-05-12 15:02 ` Achim Gottinger
@ 2008-05-12 15:26 ` Jean Delvare
4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2008-05-12 15:26 UTC (permalink / raw)
To: lm-sensors
On Mon, 12 May 2008 17:02:39 +0200, Achim Gottinger wrote:
>
> > This is how you hit the problem, but there are many other ways people
> > could hit it, such as using i2cdump, i2cget or i2cset, or even just
> > loading a hardware monitoring driver which happens to probe I2C address
> > 0x2e. While we fixed sensors-detect by now, not everyone will upgrade
> > to the new version of the script immediately, and all the other ways to
> > screw it up are still available. So, the only safe thing to do at the
> > moment is to disable the SMBus on your motherboard completely.
> >
> > In the future, we could refine this and allow access again with
> > restrictions, such as no probing allowed from kernel drivers, and no
> > access from user-space (there's some more work needed before we can do
> > that.) Or we could limit the blacklisting to specific addresses (that's
> > easier to implement).
>
> But that way you can not run it on the DFI and Sapphire boards even if
> the latest lm-sensors version is in use.
Right now there's not much interesting on the SMBus anyway on these
boards. Except for the SPD EEPROMs, none of the other chips have drivers
available. And SPD EEPROMs aren't that interesting. Or at least they
are not worth letting users break their hardware.
> How about a kernel config flag to disable this patch? Also I
No. If we decide we want to give people access to their SMBus on these
boards, then we have to do it in a safe and clean way, and that's a
long way to go. Having an config option "let me break my hardware"
doesn't sound good at all. I understand that you are into overclocking,
but me, I am into taking my responsibilities to not let users burn their
hardware.
> noted that you added infos on the website about this board.
> The DFI Lanparty UT 790FX-M2R is not mentioned there but this board is
> also affected, two users reported they had to rma
> board and/or cpu after running sensors-detect meanwile.
Can you please provide references? I will update the website.
> > I am worried that "hwmod" is too similar to "hwmon" and this will lead
> > to confusion. What about "overclock" or a similarly explicit term?
>
> I'm concerned about that similarity here also. I don't want to put the
> focus on overclocking, so
> i picked hwmod (hardware modification)
Misnomer anyway, you are not modifying the hardware with these chips
(except for the unfortunate case where you start with working hardware
and end up with dead hardware, that is.)
> for now, can use hwchange,hwshift
> or hwco (hardware change/clock over)
> also. It's easy to change this in future.
> (...)
> Is this list the right place to discuss i2c related kernel modules? I
> dont want to get off topic here.
No it's indeed not, i2c stuff should be discussed on the i2c mailing
list:
http://lists.lm-sensors.org/mailman/listinfo/i2c
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 6+ messages in thread