Skip to content

Commit a7ae819

Browse files
westeriWolfram Sang
authored andcommitted
i2c: i801: Allow ACPI SystemIO OpRegion to conflict with PCI BAR
Many Intel systems the BIOS declares a SystemIO OpRegion below the SMBus PCI device as can be seen in ACPI DSDT table from Lenovo Yoga 900: Device (SBUS) { OperationRegion (SMBI, SystemIO, (SBAR << 0x05), 0x10) Field (SMBI, ByteAcc, NoLock, Preserve) { HSTS, 8, Offset (0x02), HCON, 8, HCOM, 8, TXSA, 8, DAT0, 8, DAT1, 8, HBDR, 8, PECR, 8, RXSA, 8, SDAT, 16 } There are also bunch of AML methods that that the BIOS can use to access these fields. Most of the systems in question AML methods accessing the SMBI OpRegion are never used. Now, because of this SMBI OpRegion many systems fail to load the SMBus driver with an error looking like one below: ACPI Warning: SystemIO range 0x0000000000003040-0x000000000000305F conflicts with OpRegion 0x0000000000003040-0x000000000000304F (\_SB.PCI0.SBUS.SMBI) (20160108/utaddress-255) ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver The reason is that this SMBI OpRegion conflicts with the PCI BAR used by the SMBus driver. It turns out that we can install a custom SystemIO address space handler for the SMBus device to intercept all accesses through that OpRegion. This allows us to share the PCI BAR with the AML code if it for some reason is using it. We do not expect that this OpRegion handler will ever be called but if it is we print a warning and prevent all access from the SMBus driver itself. Link: https://bugzilla.kernel.org/show_bug.cgi?id=110041 Reported-by: Andy Lutomirski <[email protected]> Reported-by: Pali Rohár <[email protected]> Suggested-by: Rafael J. Wysocki <[email protected]> Signed-off-by: Mika Westerberg <[email protected]> Acked-by: Rafael J. Wysocki <[email protected]> Reviewed-by: Jean Delvare <[email protected]> Reviewed-by: Benjamin Tissoires <[email protected]> Tested-by: Pali Rohár <[email protected]> Tested-by: Jean Delvare <[email protected]> Signed-off-by: Wolfram Sang <[email protected]> Cc: [email protected]
1 parent af8c34c commit a7ae819

File tree

1 file changed

+96
-3
lines changed

1 file changed

+96
-3
lines changed

drivers/i2c/busses/i2c-i801.c

Lines changed: 96 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,13 @@ struct i801_priv {
245245
struct platform_device *mux_pdev;
246246
#endif
247247
struct platform_device *tco_pdev;
248+
249+
/*
250+
* If set to true the host controller registers are reserved for
251+
* ACPI AML use. Protected by acpi_lock.
252+
*/
253+
bool acpi_reserved;
254+
struct mutex acpi_lock;
248255
};
249256

250257
#define FEATURE_SMBUS_PEC (1 << 0)
@@ -718,6 +725,12 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
718725
int ret = 0, xact = 0;
719726
struct i801_priv *priv = i2c_get_adapdata(adap);
720727

728+
mutex_lock(&priv->acpi_lock);
729+
if (priv->acpi_reserved) {
730+
mutex_unlock(&priv->acpi_lock);
731+
return -EBUSY;
732+
}
733+
721734
pm_runtime_get_sync(&priv->pci_dev->dev);
722735

723736
hwpec = (priv->features & FEATURE_SMBUS_PEC) && (flags & I2C_CLIENT_PEC)
@@ -820,6 +833,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
820833
out:
821834
pm_runtime_mark_last_busy(&priv->pci_dev->dev);
822835
pm_runtime_put_autosuspend(&priv->pci_dev->dev);
836+
mutex_unlock(&priv->acpi_lock);
823837
return ret;
824838
}
825839

@@ -1257,6 +1271,83 @@ static void i801_add_tco(struct i801_priv *priv)
12571271
priv->tco_pdev = pdev;
12581272
}
12591273

1274+
#ifdef CONFIG_ACPI
1275+
static acpi_status
1276+
i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits,
1277+
u64 *value, void *handler_context, void *region_context)
1278+
{
1279+
struct i801_priv *priv = handler_context;
1280+
struct pci_dev *pdev = priv->pci_dev;
1281+
acpi_status status;
1282+
1283+
/*
1284+
* Once BIOS AML code touches the OpRegion we warn and inhibit any
1285+
* further access from the driver itself. This device is now owned
1286+
* by the system firmware.
1287+
*/
1288+
mutex_lock(&priv->acpi_lock);
1289+
1290+
if (!priv->acpi_reserved) {
1291+
priv->acpi_reserved = true;
1292+
1293+
dev_warn(&pdev->dev, "BIOS is accessing SMBus registers\n");
1294+
dev_warn(&pdev->dev, "Driver SMBus register access inhibited\n");
1295+
1296+
/*
1297+
* BIOS is accessing the host controller so prevent it from
1298+
* suspending automatically from now on.
1299+
*/
1300+
pm_runtime_get_sync(&pdev->dev);
1301+
}
1302+
1303+
if ((function & ACPI_IO_MASK) == ACPI_READ)
1304+
status = acpi_os_read_port(address, (u32 *)value, bits);
1305+
else
1306+
status = acpi_os_write_port(address, (u32)*value, bits);
1307+
1308+
mutex_unlock(&priv->acpi_lock);
1309+
1310+
return status;
1311+
}
1312+
1313+
static int i801_acpi_probe(struct i801_priv *priv)
1314+
{
1315+
struct acpi_device *adev;
1316+
acpi_status status;
1317+
1318+
adev = ACPI_COMPANION(&priv->pci_dev->dev);
1319+
if (adev) {
1320+
status = acpi_install_address_space_handler(adev->handle,
1321+
ACPI_ADR_SPACE_SYSTEM_IO, i801_acpi_io_handler,
1322+
NULL, priv);
1323+
if (ACPI_SUCCESS(status))
1324+
return 0;
1325+
}
1326+
1327+
return acpi_check_resource_conflict(&priv->pci_dev->resource[SMBBAR]);
1328+
}
1329+
1330+
static void i801_acpi_remove(struct i801_priv *priv)
1331+
{
1332+
struct acpi_device *adev;
1333+
1334+
adev = ACPI_COMPANION(&priv->pci_dev->dev);
1335+
if (!adev)
1336+
return;
1337+
1338+
acpi_remove_address_space_handler(adev->handle,
1339+
ACPI_ADR_SPACE_SYSTEM_IO, i801_acpi_io_handler);
1340+
1341+
mutex_lock(&priv->acpi_lock);
1342+
if (priv->acpi_reserved)
1343+
pm_runtime_put(&priv->pci_dev->dev);
1344+
mutex_unlock(&priv->acpi_lock);
1345+
}
1346+
#else
1347+
static inline int i801_acpi_probe(struct i801_priv *priv) { return 0; }
1348+
static inline void i801_acpi_remove(struct i801_priv *priv) { }
1349+
#endif
1350+
12601351
static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
12611352
{
12621353
unsigned char temp;
@@ -1274,6 +1365,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
12741365
priv->adapter.dev.parent = &dev->dev;
12751366
ACPI_COMPANION_SET(&priv->adapter.dev, ACPI_COMPANION(&dev->dev));
12761367
priv->adapter.retries = 3;
1368+
mutex_init(&priv->acpi_lock);
12771369

12781370
priv->pci_dev = dev;
12791371
switch (dev->device) {
@@ -1336,10 +1428,8 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
13361428
return -ENODEV;
13371429
}
13381430

1339-
err = acpi_check_resource_conflict(&dev->resource[SMBBAR]);
1340-
if (err) {
1431+
if (i801_acpi_probe(priv))
13411432
return -ENODEV;
1342-
}
13431433

13441434
err = pcim_iomap_regions(dev, 1 << SMBBAR,
13451435
dev_driver_string(&dev->dev));
@@ -1348,6 +1438,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
13481438
"Failed to request SMBus region 0x%lx-0x%Lx\n",
13491439
priv->smba,
13501440
(unsigned long long)pci_resource_end(dev, SMBBAR));
1441+
i801_acpi_remove(priv);
13511442
return err;
13521443
}
13531444

@@ -1412,6 +1503,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
14121503
err = i2c_add_adapter(&priv->adapter);
14131504
if (err) {
14141505
dev_err(&dev->dev, "Failed to add SMBus adapter\n");
1506+
i801_acpi_remove(priv);
14151507
return err;
14161508
}
14171509

@@ -1438,6 +1530,7 @@ static void i801_remove(struct pci_dev *dev)
14381530

14391531
i801_del_mux(priv);
14401532
i2c_del_adapter(&priv->adapter);
1533+
i801_acpi_remove(priv);
14411534
pci_write_config_byte(dev, SMBHSTCFG, priv->original_hstcfg);
14421535

14431536
platform_device_unregister(priv->tco_pdev);

0 commit comments

Comments
 (0)