hwmon: (asus_atk0110) Don't load if ACPI
Subsystems (3)
PATCH 1/1 hwmon: (asus_atk0110) Don't load if ACPI
When the user passes the kernel parameter acpi_enforce_resources=lax, the ACPI resources are no longer protected, so a native driver can make use of them. In that case, we do not want the asus_atk0110 to be loaded. Unfortunately, this driver loads automatically due to its MODULE_DEVICE_TABLE, so the user ends up with two drivers loaded for the same device - this is bad.
So I suggest that we prevent the asus_atk0110 driver from loading if acpi_enforce_resources=lax.
---
Luca, what do you think? I had the idea to write this patch after seeing this bug report:
https://bugzilla.novell.com/show_bug.cgi?idX0988
I'm not sure if the driver conflict is the root cause of the problem, but it certainly did not help.
Another approach would be to hide the ATK0110 ACPI device itself, but I don't know if it is possible?
drivers/acpi/osl.c | 9 +++++++++ drivers/hwmon/asus_atk0110.c | 7 +++++++ include/linux/acpi.h | 2 ++ 3 files changed, 18 insertions(+)
drivers/acpi/osl.c +9 -0
@@ -1206,6 +1206,15 @@ int acpi_check_mem_region(resource_size_EXPORT_SYMBOL (acpi_check_mem_region);/* +* Let drivers know whether the resource checks are effective +*/ +int acpi_resources_are_enforced (void ) +{ +return acpi_enforce_resources =ENFORCE_RESOURCES_STRICT ; +} +EXPORT_SYMBOL (acpi_resources_are_enforced); + +/* * Acquire a spinlock. * * handle is a pointer to the spinlock_t.
drivers/hwmon/asus_atk0110.c +7 -0
@@ -1406,6 +1406,13 @@ static int __init atk0110_init(void) {int ret; +/* Make sure it's safe to access the device through ACPI */ +if (!acpi_resources_are_enforced ()) { +pr_err ("atk: Resources not safely usable due to " +"acpi_enforce_resources kernel parameter\n" ); +return -EBUSY ; + } + ret =acpi_bus_register_driver (&atk_driver);if (ret)pr_info ("atk: acpi_bus_register_driver failed: %d\n" , ret);
include/linux/acpi.h +2 -0
@@ -247,6 +247,8 @@ int acpi_check_region(resource_size_t stint acpi_check_mem_region (resource_size_t start,resource_size_t n,const char *name); +int acpi_resources_are_enforced (void ); +#ifdef CONFIG_PM_SLEEP void __init acpi_no_s4_hw_signature (void );void __init acpi_old_suspend_ordering (void );
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Quoted 7 lines · Jean Delvare
When the user passes the kernel parameter acpi_enforce_resources=lax, the ACPI resources are no longer protected, so a native driver can make use of them. In that case, we do not want the
asus_atk0110to be loaded. Unfortunately, this driver loads automatically due to itsMODULE_DEVICE_TABLE, so the user ends up with two drivers loaded for the same device - this is bad.
Quoted 6 lines
So I suggest that we prevent the
asus_atk0110driver from loading if acpi_enforce_resources=lax.Signed-off-by Jean Delvare <[email protected]>Cc Luca Tettamanti <[email protected]>Cc Len Brown <[email protected]>
Quoted 6 lines
---
Luca, what do you think? I had the idea to write this patch after seeing this bug report:
https://bugzilla.novell.com/show_bug.cgi?idX0988
I'm not sure if the driver conflict is the root cause of the problem, but it certainly did not help.
The patch look good.
Quoted 2 lines
Another approach would be to hide the ATK0110 ACPI device itself, but I don't know if it is possible?
AFAIK it's not possible.
Luca
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
In reply to Jean DelvareACPI people,
Do you have any objection to the patch below, or can I push it upstream?
Quoted 75 lines · Jean Delvare
When the user passes the kernel parameter acpi_enforce_resources=lax, the ACPI resources are no longer protected, so a native driver can make use of them. In that case, we do not want the
asus_atk0110to be loaded. Unfortunately, this driver loads automatically due to itsMODULE_DEVICE_TABLE, so the user ends up with two drivers loaded for the same device - this is bad.So I suggest that we prevent the
asus_atk0110driver from loading if acpi_enforce_resources=lax.Signed-off-by Jean Delvare <[email protected]>Cc Luca Tettamanti <[email protected]>Cc Len Brown <[email protected]>---
Luca, what do you think? I had the idea to write this patch after seeing this bug report:
https://bugzilla.novell.com/show_bug.cgi?idX0988
I'm not sure if the driver conflict is the root cause of the problem, but it certainly did not help.Another approach would be to hide the ATK0110 ACPI device itself, but I don't know if it is possible?
drivers/acpi/osl.c | 9 +++++++++ drivers/hwmon/asus_atk0110.c | 7 +++++++ include/linux/acpi.h | 2 ++ 3 files changed, 18 insertions(+)drivers/acpi/osl.c +9
-0@@ -1206,6 +1206,15 @@ int acpi_check_mem_region(resource_size_EXPORT_SYMBOL (acpi_check_mem_region);/* +* Let drivers know whether the resource checks are effective +*/ +int acpi_resources_are_enforced (void ) +{ +return acpi_enforce_resources =ENFORCE_RESOURCES_STRICT ; +} +EXPORT_SYMBOL (acpi_resources_are_enforced); + +/* * Acquire a spinlock. * * handle is a pointer to the spinlock_t. drivers/hwmon/asus_atk0110.c +7
-0@@ -1406,6 +1406,13 @@ static int __init atk0110_init(void) {int ret; +/* Make sure it's safe to access the device through ACPI */ +if (!acpi_resources_are_enforced ()) { +pr_err ("atk: Resources not safely usable due to " +"acpi_enforce_resources kernel parameter\n" ); +return -EBUSY ; + } + ret =acpi_bus_register_driver (&atk_driver);if (ret)pr_info ("atk: acpi_bus_register_driver failed: %d\n" , ret);include/linux/acpi.h +2
-0@@ -247,6 +247,8 @@ int acpi_check_region(resource_size_t stint acpi_check_mem_region (resource_size_t start,resource_size_t n,const char *name); +int acpi_resources_are_enforced (void ); +#ifdef CONFIG_PM_SLEEP void __init acpi_no_s4_hw_signature (void );void __init acpi_old_suspend_ordering (void );
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Quoted 3 lines · Jean Delvare
ACPI people,
Do you have any objection to the patch below, or can I push it upstream?
If you pass that boot option, you're already indicating that you don't care about multiple things accessing the same resources simultaneously. I think it's reasonable to leave things up to the user to configure at that point.
Signature · 6 lines
Matthew Garrett | [email protected] _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Hi Matthew,
Quoted 9 lines · Matthew Garrett
Quoted 3 lines · Jean Delvare
ACPI people,
Do you have any objection to the patch below, or can I push it upstream?
If you pass that boot option, you're already indicating that you don't care about multiple things accessing the same resources simultaneously. I think it's reasonable to leave things up to the user to configure at that point.
The asus_atk0110 driver loads automatically. The user doesn't configure anything, and it just loads. That's the problem. If the driver didn't load automatically, I wouldn't care. If you know of any way to prevent the asus_atk0110 driver from auto-loading as soon as acpi_enforce_resouce=lax or =no is passed, I am fine with this as well. This is what I wanted to do originally, but I couldn't find a way to do it, which is why I went for the more radical option of preventing the asus_atk0110 driver from loading.
Thanks,
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Quoted 8 lines · Jean Delvare
The
asus_atk0110driver loads automatically. The user doesn't configure anything, and it just loads. That's the problem. If the driver didn't load automatically, I wouldn't care. If you know of any way to prevent theasus_atk0110driver from auto-loading as soon as acpi_enforce_resouce=lax or =no is passed, I am fine with this as well. This is what I wanted to do originally, but I couldn't find a way to do it, which is why I went for the more radical option of preventing theasus_atk0110driver from loading.
echo blacklist asus_atk0110 >/etc/modprobe.d/hwmon_blacklist.conf
Signature · 6 lines
Matthew Garrett | [email protected] _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Quoted 11 lines · Matthew Garrett
Quoted 8 lines · Jean Delvare
The
asus_atk0110driver loads automatically. The user doesn't configure anything, and it just loads. That's the problem. If the driver didn't load automatically, I wouldn't care. If you know of any way to prevent theasus_atk0110driver from auto-loading as soon as acpi_enforce_resouce=lax or =no is passed, I am fine with this as well. This is what I wanted to do originally, but I couldn't find a way to do it, which is why I went for the more radical option of preventing theasus_atk0110driver from loading.
Quoted 1 line
echo blacklist
asus_atk0110>/etc/modprobe.d/hwmon_blacklist.conf
Sure, but why do you insist on having the user configure this manually when we can automate this at the kernel level? When acpi_enforce_resouce=yes, the kernel doesn't let non-ACPI driver be loaded, so I fail to see why we let ACPI drivers (for which we also have native drivers) load when acpi_enforce_resouce=no.
Or is there something really wrong with my patch?
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Quoted 5 lines · Jean Delvare
Sure, but why do you insist on having the user configure this manually when we can automate this at the kernel level? When acpi_enforce_resouce=yes, the kernel doesn't let non-ACPI driver be loaded, so I fail to see why we let ACPI drivers (for which we also have native drivers) load when acpi_enforce_resouce=no.
Because the situation with the asus driver loaded isn't obviously any worse than not having it loaded. The user is telling us that they're happy with racy access to their hwmon hardware. Automatically blocking the loading of the ACPI driver does nothing other than imply to the user that things are safe, when in reality they're anything but.
Signature · 6 lines
Matthew Garrett | [email protected] _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Luca Tettamanti
maintainerRe: [lm-sensors] [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Quoted 11 lines · Matthew Garrett
Quoted 5 lines · Jean Delvare
Sure, but why do you insist on having the user configure this manually when we can automate this at the kernel level? When acpi_enforce_resouce=yes, the kernel doesn't let non-ACPI driver be loaded, so I fail to see why we let ACPI drivers (for which we also have native drivers) load when acpi_enforce_resouce=no.
Because the situation with the asus driver loaded isn't obviously any worse than not having it loaded. The user is telling us that they're happy with racy access to their hwmon hardware.
"lax" option is typically used when the user wants to load the native driver; in this case we know that it's not safe to load asus driver.
Luca
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Quoted 7 lines · Luca Tettamanti
Quoted 3 lines · Matthew Garrett
Because the situation with the asus driver loaded isn't obviously any worse than not having it loaded. The user is telling us that they're happy with racy access to their hwmon hardware.
"lax" option is typically used when the user wants to load the native driver; in this case we know that it's not safe to load asus driver.
It's not safe to load the native driver, full stop. Not loading the asus driver doesn't alter that.
Signature · 6 lines
Matthew Garrett | [email protected] _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Quoted 11 lines · Matthew Garrett
Quoted 7 lines · Luca Tettamanti
Quoted 3 lines · Matthew Garrett
Because the situation with the asus driver loaded isn't obviously any worse than not having it loaded. The user is telling us that they're happy with racy access to their hwmon hardware.
"lax" option is typically used when the user wants to load the native driver; in this case we know that it's not safe to load asus driver.
It's not safe to load the native driver, full stop. Not loading the asus driver doesn't alter that.
It definitely does. At least on some of the affected boards, no access to the I/O area in question is done at all as long as the asus_atk0110 driver is not loaded. So it is actually safe to pass acpi_enforce_resources=lax and not load the asus_atk0110 driver. And people are actually doing this because native drivers can control the speed of their fans while the asus_atk0110 driver can't. So they see the current situation as a regression.
Again, can you please point to problems my patch has?
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Quoted 3 lines · Jean Delvare
It definitely does. At least on some of the affected boards, no access to the I/O area in question is done at all as long as the
asus_atk0110driver is not loaded.
You've audited the system management code?
Signature · 6 lines
Matthew Garrett | [email protected] _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Quoted 7 lines · Matthew Garrett
Quoted 3 lines · Jean Delvare
It definitely does. At least on some of the affected boards, no access to the I/O area in question is done at all as long as the
asus_atk0110driver is not loaded.You've audited the system management code?
No, I did not. But you did not review the system management code of all computers out there either, and you are still writing kernel code that could be broken by it. So what's the point?
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Quoted 7 lines · Jean Delvare
Quoted 6 lines · Matthew Garrett
Quoted 3 lines · Jean Delvare
It definitely does. At least on some of the affected boards, no access to the I/O area in question is done at all as long as the
asus_atk0110driver is not loaded.
Quoted 5 lines
Quoted 1 line
You've audited the system management code?
No, I did not. But you did not review the system management code of all computers out there either, and you are still writing kernel code that could be broken by it. So what's the point?
If the firmware tells us that a resource range using indexed access is reserved, it's not safe to assume that the firmware isn't using it.
Signature · 6 lines
Matthew Garrett | [email protected] _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
Quoted 2 lines · Matthew Garrett
If the firmware tells us that a resource range using indexed access is reserved, it's not safe to assume that the firmware isn't using it.
Whether it is safe of not is irrelevant. The user is telling us he/she _will_ use it nevertheless. So what is wrong with doing our best to avoid problems?
The user paid for the hardware, and firmwares are often crappy. You know that better than anyone else, having worked on ACPI for some time now.
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [PATCH] hwmon: (asus_atk0110) Don't load if ACPI resources aren't enforced
In reply to Matthew GarrettQuoted 11 lines · Matthew Garrett
Quoted 5 lines · Jean Delvare
Sure, but why do you insist on having the user configure this manually when we can automate this at the kernel level? When acpi_enforce_resouce=yes, the kernel doesn't let non-ACPI driver be loaded, so I fail to see why we let ACPI drivers (for which we also have native drivers) load when acpi_enforce_resouce=no.
Because the situation with the asus driver loaded isn't obviously any worse than not having it loaded. The user is telling us that they're happy with racy access to their hwmon hardware.
No, they are not saying that. They are saying: I want to let native drivers access ACPI-reserved I/O ports. That's all they are saying. Attaching a single interpretation to this is incorrect. They might do this because their BIOS improperly reserves ports it doesn't use. Or because they prefer using a native driver over an ACPI driver for a given chip. Or indeed because they want to crash their machine. Just because it can't be called generally safe (and in all honesty is unsafe in many cases, quite possibly the majority) doesn't mean there are no cases where it makes sense. Hell, we lived without ACPI resource reservation until kernel 2.6.32 and without the asus_atk0110 driver and most people were totally happy with the situation.
Quoted 3 lines
Automatically blocking
the loading of the ACPI driver does nothing other than imply to the user that things are safe,
Again, no, it doesn't imply that. It implies that loading the asus_atk0110 driver together with a native driver for the same device is very bad and should never be done, period.
Quoted 1 line
when in reality they're anything but.
They sometimes are, this is the point.
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors