diff mbox series

i2c: i801: Add module parameter to inhibit BIOS access

Message ID 20220111233151.8484-1-alexh@vpitech.com
State New
Headers show
Series i2c: i801: Add module parameter to inhibit BIOS access | expand

Commit Message

Alex Henrie Jan. 11, 2022, 11:31 p.m. UTC
This parameter can only be set before the module is loaded (e.g. by
passing i2c_i801.block_acpi=1 on the kernel command line).

Signed-off-by: Alex Henrie <alexh@vpitech.com>
---
 drivers/i2c/busses/i2c-i801.c | 63 +++++++++++++++++++++++++++--------
 1 file changed, 49 insertions(+), 14 deletions(-)

Comments

Alex Henrie Jan. 19, 2022, 7:24 p.m. UTC | #1
On Wed, 19 Jan 2022 19:43:00 +0100
Wolfram Sang <wsa@kernel.org> wrote:

> 
> > Jean, please feel free to disregard my patch and to commit Hector's
> > with "Tested-by: Alex Henrie <alexh@vpitech.com>".
> 
> Please respnd to the mail directly with the Tested-by tag. We have
> tooling which collects tags for us per patch. Makes a maintainers life a
> lot easier.

I wasn't subscribed to the mailing list when that patch was sent, and
without my own copy of the patch email I don't have an easy way to
reply with the correct In-Reply-To header. Is a reply with a correct
Subject header enough for the automated tools?

-Alex
Wolfram Sang Jan. 19, 2022, 8:22 p.m. UTC | #2
> I wasn't subscribed to the mailing list when that patch was sent, and
> without my own copy of the patch email I don't have an easy way to
> reply with the correct In-Reply-To header. Is a reply with a correct
> Subject header enough for the automated tools?

I see. I'll bounce the message to you.
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 41446f9cc52d..615fd1185b61 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -288,6 +288,11 @@  struct i801_priv {
 	 * ACPI AML use. Protected by acpi_lock.
 	 */
 	bool acpi_reserved;
+	/*
+	 * If set to true ACPI AML tried to use SMBus but block_acpi was
+	 * set. Protected by acpi_lock.
+	 */
+	bool acpi_blocked;
 	struct mutex acpi_lock;
 };
 
@@ -320,6 +325,11 @@  MODULE_PARM_DESC(disable_features, "Disable selected driver features:\n"
 	"\t\t  0x10  don't use interrupts\n"
 	"\t\t  0x20  disable SMBus Host Notify ");
 
+static bool block_acpi;
+module_param(block_acpi, bool, S_IRUGO);
+MODULE_PARM_DESC(block_acpi, "Prevent ACPI AML from accessing SMBus. "
+	"[0] = allow ACPI access, 1 = deny ACPI access");
+
 /* Make sure the SMBus host is ready to start transmitting.
    Return 0 if it is, -EBUSY if it is not. */
 static int i801_check_pre(struct i801_priv *priv)
@@ -1616,23 +1626,48 @@  i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits,
 	acpi_status status;
 
 	/*
-	 * Once BIOS AML code touches the OpRegion we warn and inhibit any
-	 * further access from the driver itself. This device is now owned
-	 * by the system firmware.
+	 * If BIOS AML code tries to touches the OpRegion we have two options:
+	 * Warn and inhibit any further access from the driver, or warn and
+	 * inhibit all access from the BIOS.
 	 */
 	mutex_lock(&priv->acpi_lock);
 
-	if (!priv->acpi_reserved && i801_acpi_is_smbus_ioport(priv, address)) {
-		priv->acpi_reserved = true;
-
-		dev_warn(&pdev->dev, "BIOS is accessing SMBus registers\n");
-		dev_warn(&pdev->dev, "Driver SMBus register access inhibited\n");
-
-		/*
-		 * BIOS is accessing the host controller so prevent it from
-		 * suspending automatically from now on.
-		 */
-		pm_runtime_get_sync(&pdev->dev);
+	if (i801_acpi_is_smbus_ioport(priv, address)) {
+		if (block_acpi) {
+			/*
+			 * Refuse to allow the BIOS to use SMBus. SMBus does
+			 * have a lock bit in the status register that in theory
+			 * can be used to safely share the SMBus between the
+			 * BIOS and the kernel, but some badly behaved BIOS
+			 * implementations don't use it. In that case, the only
+			 * way to ensure continued safe access from the driver
+			 * is to cripple the BIOS.
+			 */
+			if (!priv->acpi_blocked) {
+				dev_warn(&pdev->dev,
+					 "BIOS tried to access SMBus registers\n");
+				dev_warn(&pdev->dev,
+					 "BIOS SMBus register access inhibited\n");
+				priv->acpi_blocked = true;
+			}
+			mutex_unlock(&priv->acpi_lock);
+			return -EPERM;
+		}
+		if (!priv->acpi_reserved) {
+			/* This device is now owned by the system firmware. */
+			priv->acpi_reserved = true;
+
+			dev_warn(&pdev->dev,
+				 "BIOS is accessing SMBus registers\n");
+			dev_warn(&pdev->dev,
+				 "Driver SMBus register access inhibited\n");
+
+			/*
+			 * BIOS is accessing the host controller so prevent it
+			 * from suspending automatically from now on.
+			 */
+			pm_runtime_get_sync(&pdev->dev);
+		}
 	}
 
 	if ((function & ACPI_IO_MASK) == ACPI_READ)