Re: [PATCH v3] i2c: i801: Safely share SMBus with BIOS/ACPI

Re: [PATCH v3] i2c: i801: Safely share SMBus with BIOS/ACPI

[Alex Henrie]

Dear kernel developers,

I am having a similar problem, but unfortunately this patch doesn't
work for me (I get the error "BIOS uses SMBus unsafely"). Would it be
acceptable to add a module parameter to allow access to the SMBus, even
if the BIOS is using it? I realize that this is not a good idea in
general, but I believe it is safe in my particular case, and I don't
see any other way to solve my problem.

-Alex

Re: [PATCH v3] i2c: i801: Safely share SMBus with BIOS/ACPI

[Hector Martin]

On 17/12/2021 09.31, Alex Henrie wrote:
> Dear kernel developers,
> 
> I am having a similar problem, but unfortunately this patch doesn't
> work for me (I get the error "BIOS uses SMBus unsafely"). Would it be
> acceptable to add a module parameter to allow access to the SMBus, even
> if the BIOS is using it? I realize that this is not a good idea in
> general, but I believe it is safe in my particular case, and I don't
> see any other way to solve my problem.
> 
> -Alex
> 

How is this safe in your case? If the BIOS is using SMBus without
locking it, and may do so at any time, then there's no way to safely use
it from Linux.

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

Re: [External] Re: [PATCH v3] i2c: i801: Safely share SMBus with BIOS/ACPI

[Alex Henrie]

On Fri, 17 Dec 2021 16:41:05 +0900
Hector Martin <marcan@marcan.st> wrote:

> On 17/12/2021 09.31, Alex Henrie wrote:
> > I am having a similar problem, but unfortunately this patch doesn't
> > work for me (I get the error "BIOS uses SMBus unsafely"). Would it be
> > acceptable to add a module parameter to allow access to the SMBus, even
> > if the BIOS is using it? I realize that this is not a good idea in
> > general, but I believe it is safe in my particular case, and I don't
> > see any other way to solve my problem.
> 
> How is this safe in your case? If the BIOS is using SMBus without
> locking it, and may do so at any time, then there's no way to safely use
> it from Linux.

The BIOS appears to access the SMBus during the first few minutes after
boot, and then it stops using it. So "safe" may not be the right word,
but in my case, it seems to work OK to use the SMBus as long as the
uptime is greater than a few minutes.

-Alex

Re: [External] Re: [PATCH v3] i2c: i801: Safely share SMBus with BIOS/ACPI

[Hector Martin]

On 18/12/2021 11.51, Alex Henrie wrote:
> On Fri, 17 Dec 2021 16:41:05 +0900
> Hector Martin <marcan@marcan.st> wrote:
> 
>> On 17/12/2021 09.31, Alex Henrie wrote:
>>> I am having a similar problem, but unfortunately this patch doesn't
>>> work for me (I get the error "BIOS uses SMBus unsafely"). Would it be
>>> acceptable to add a module parameter to allow access to the SMBus, even
>>> if the BIOS is using it? I realize that this is not a good idea in
>>> general, but I believe it is safe in my particular case, and I don't
>>> see any other way to solve my problem.
>>
>> How is this safe in your case? If the BIOS is using SMBus without
>> locking it, and may do so at any time, then there's no way to safely use
>> it from Linux.
> 
> The BIOS appears to access the SMBus during the first few minutes after
> boot, and then it stops using it. So "safe" may not be the right word,
> but in my case, it seems to work OK to use the SMBus as long as the
> uptime is greater than a few minutes.
> 
> -Alex
> 

Linux will probe the SMBus on startup, so that still seems like a rather
fragile situation. You'd have to blacklist the module and load it
separately a few minutes after boot, or something like that...

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

Re: [PATCH v3] i2c: i801: Safely share SMBus with BIOS/ACPI

[Alex Henrie]

On Sat, 18 Dec 2021 12:39:11 +0900
Hector Martin <marcan@marcan.st> wrote:

> On 18/12/2021 11.51, Alex Henrie wrote:
> > On Fri, 17 Dec 2021 16:41:05 +0900
> > Hector Martin <marcan@marcan.st> wrote:
> > 
> >> On 17/12/2021 09.31, Alex Henrie wrote:
> >>> I am having a similar problem, but unfortunately this patch doesn't
> >>> work for me (I get the error "BIOS uses SMBus unsafely"). Would it be
> >>> acceptable to add a module parameter to allow access to the SMBus, even
> >>> if the BIOS is using it? I realize that this is not a good idea in
> >>> general, but I believe it is safe in my particular case, and I don't
> >>> see any other way to solve my problem.
> >>
> >> How is this safe in your case? If the BIOS is using SMBus without
> >> locking it, and may do so at any time, then there's no way to safely use
> >> it from Linux.
> > 
> > The BIOS appears to access the SMBus during the first few minutes after
> > boot, and then it stops using it. So "safe" may not be the right word,
> > but in my case, it seems to work OK to use the SMBus as long as the
> > uptime is greater than a few minutes.
> 
> Linux will probe the SMBus on startup, so that still seems like a rather
> fragile situation. You'd have to blacklist the module and load it
> separately a few minutes after boot, or something like that...

It's awful, I agree. I was just hoping to find a slightly less awful
solution than patching the kernel to remove the BIOS access check,
which is what I currently have to do. It seems to me that if multiple
people are in this situation, it might make sense to add a module
parameter to disable the check.

-Alex

Re: [PATCH v3] i2c: i801: Safely share SMBus with BIOS/ACPI

[Alex Henrie]

Hi Hector,

I had an idea today that you might like better. In your proposed patch
to share the SMBus with the BIOS, the kernel will lock itself out of
the SMBus if the BIOS uses it unsafely. What if we add a module
parameter to instead return -EPERM in i801_acpi_io_handler, effectively
locking the BIOS out of the SMBus instead of locking the kernel out? I
tried hacking that behavior in and so far it's working well on my
troublesome machine.

-Alex

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 8020b6b97..442eef67a 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1629,6 +1629,9 @@ i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits,
 	mutex_lock(&priv->acpi_lock);
 
 	if (!priv->acpi_reserved && i801_acpi_is_smbus_ioport(priv, address)) {
+		mutex_unlock(&priv->acpi_lock);
+		return -EPERM;
+
 		priv->acpi_reserved = true;
 
 		dev_warn(&pdev->dev, "BIOS is accessing SMBus registers\n");

Re: [PATCH v3] i2c: i801: Safely share SMBus with BIOS/ACPI

[Hector Martin]

On 2022/01/05 10:37, Alex Henrie wrote:
> Hi Hector,
> 
> I had an idea today that you might like better. In your proposed patch
> to share the SMBus with the BIOS, the kernel will lock itself out of
> the SMBus if the BIOS uses it unsafely. What if we add a module
> parameter to instead return -EPERM in i801_acpi_io_handler, effectively
> locking the BIOS out of the SMBus instead of locking the kernel out? I
> tried hacking that behavior in and so far it's working well on my
> troublesome machine.
> 
> -Alex
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 8020b6b97..442eef67a 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1629,6 +1629,9 @@ i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits,
>  	mutex_lock(&priv->acpi_lock);
>  
>  	if (!priv->acpi_reserved && i801_acpi_is_smbus_ioport(priv, address)) {
> +		mutex_unlock(&priv->acpi_lock);
> +		return -EPERM;
> +
>  		priv->acpi_reserved = true;
>  
>  		dev_warn(&pdev->dev, "BIOS is accessing SMBus registers\n");
> 

I think this makes more sense. It would be up to users to choose to
cripple whatever the BIOS does, of course (the reason why I sent this
patch in in the first place was because I needed the ACPI SMBus routines
to work on my machine to make backlight controls work).

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

Re: [External] Re: [PATCH v3] i2c: i801: Safely share SMBus with BIOS/ACPI

[Jean Delvare]

On Fri, 17 Dec 2021 19:51:28 -0700, Alex Henrie wrote:
> The BIOS appears to access the SMBus during the first few minutes after
> boot, and then it stops using it. So "safe" may not be the right word,
> but in my case, it seems to work OK to use the SMBus as long as the
> uptime is greater than a few minutes.

This is a fairly uncommon pattern. Do you have an idea, which device
and registers the BIOS is accessing? Did you try tracing it?

-- 
Jean Delvare
SUSE L3 Support