Re: [PATCH] platform/x86: Export LPC attributes for the system SPI chip

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 11 May 2020 at 11:45, Mika Westerberg
<mika.westerberg@xxxxxxxxxxxxxxx> wrote:
> I think you may want to look at drivers/mfd/lpc_ich.c and see if some of
> this can be placed there. It is the "LPC" driver that binds to the
> LPC/eSPI PCI device so it already has at least some of these PCI IDs.

Ahh, that's useful, thanks. I've attached something based on lpc_ich
that works, although there are a few things needing discussion:

* Some of the reported eSPI IDs are missing, and I can't easily find
references to them in the official Intel specifications. Do you need
me to hunt them down, or can I add DIDs without referencing a document
number? I can certainly split out the new DIDs and the securityfs
stuff when this patchset looks half-acceptable.

* Do you want the CONFIG_SECURITY functionality put in a different
file, perhaps with a different Kconfig entry? e.g. LPC_SCH_SECURITYFS
-- if so, how do you want the hooks implemented? Declare a dummy
lpc_ich_init_securityfs() if there is no support for securityfs like
iommu does? Put the securityfs dentries static in this new file rather
than in the lpc_ich_priv struct? Any advice welcome.

* My hardware seems to not set RCBA and so res->start never gets
defined.I don't think it's a problem from my naive point of view, but
the -ENODEV is presumably there for a reason.

If you want the patch inline, that's fine too, please just ask and I
can resend. Thanks for your help so far, and sorry for all the silly
mistakes -- I'm new to all this kernel stuff.

Richard.
From c714162f6ffed419e538fadedfb27b735e882b03 Mon Sep 17 00:00:00 2001
From: Richard Hughes <richard@xxxxxxxxxxx>
Date: Mon, 11 May 2020 14:19:27 +0100
Subject: [PATCH] mfd: Export LPC attributes for the system SPI chip

Export standard SPI-specific config values from various LPC controllers.
This allows userspace components such as fwupd to verify the most basic SPI
protections are set correctly. For instance, checking BIOSWE is disabled
and BLE is enabled.

Exporting these values from the kernel allows us to report the security
level of the platform without rebooting and running an EFI binary like
chipsec.

Signed-off-by: Richard Hughes <richard@xxxxxxxxxxx>
---
 Documentation/ABI/testing/sysfs-security-spi |  17 ++
 drivers/mfd/lpc_ich.c                        | 155 ++++++++++++++++++-
 include/linux/platform_data/intel-spi.h      |   6 +-
 3 files changed, 172 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-security-spi

diff --git a/Documentation/ABI/testing/sysfs-security-spi b/Documentation/ABI/testing/sysfs-security-spi
new file mode 100644
index 000000000000..ee867b1366f9
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-security-spi
@@ -0,0 +1,17 @@
+What:           /sys/kernel/security/spi/bioswe
+Date:           May 2020
+Contact:        platform-driver-x86@xxxxxxxxxxxxxxx
+Description:    If the system firmware set BIOS Write Enable.
+		0: writes disabled, 1: writes enabled.
+
+What:           /sys/kernel/security/spi/ble
+Date:           May 2020
+Contact:        platform-driver-x86@xxxxxxxxxxxxxxx
+Description:    If the system firmware set Bios Lock Enable.
+		0: SMM lock disabled, 1: SMM lock enabled.
+
+What:           /sys/kernel/security/spi/smm_bwp
+Date:           May 2020
+Contact:        platform-driver-x86@xxxxxxxxxxxxxxx
+Description:    If the system firmware set SMM Bios Write Protect.
+		0: writes disabled unless in SMM, 1: writes enabled.
diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
index 3bbb29a7e7a5..ff1b03f67307 100644
--- a/drivers/mfd/lpc_ich.c
+++ b/drivers/mfd/lpc_ich.c
@@ -33,6 +33,8 @@
  *	document number 322169-001, 322170-003: 5 Series, 3400 Series (PCH)
  *	document number 320066-003, 320257-008: EP80597 (IICH)
  *	document number 324645-001, 324646-001: Cougar Point (CPT)
+ *	document number 332690-006, 332691-003: C230 (CPT)
+ *	document number 337867-003, 337868-002: Cannon Point (PCH)
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -46,6 +48,10 @@
 #include <linux/mfd/lpc_ich.h>
 #include <linux/platform_data/itco_wdt.h>
 
+#ifdef CONFIG_SECURITY
+#include <linux/security.h>
+#endif
+
 #define ACPIBASE		0x40
 #define ACPIBASE_GPE_OFF	0x28
 #define ACPIBASE_GPE_END	0x2f
@@ -68,6 +74,8 @@
 #define SPIBASE_LPT_SZ		512
 #define BCR			0xdc
 #define BCR_WPD			BIT(0)
+#define BCR_BLE			BIT(1)
+#define BCR_SMM_BWP		BIT(5)
 
 #define SPIBASE_APL_SZ		4096
 
@@ -93,6 +101,13 @@ struct lpc_ich_priv {
 	int abase_save;		/* Cached ACPI base value */
 	int actrl_pbase_save;		/* Cached ACPI control or PMC base value */
 	int gctrl_save;		/* Cached GPIO control value */
+
+#ifdef CONFIG_SECURITY
+	struct dentry *spi_dir;		/* SecurityFS entries */
+	struct dentry *spi_bioswe;
+	struct dentry *spi_ble;
+	struct dentry *spi_smm_bwp;
+#endif
 };
 
 static struct resource wdt_ich_res[] = {
@@ -221,6 +236,9 @@ enum lpc_chipsets {
 	LPC_APL,	/* Apollo Lake SoC */
 	LPC_GLK,	/* Gemini Lake SoC */
 	LPC_COUGARMOUNTAIN,/* Cougar Mountain SoC*/
+	LPC_SPT,	/* Sunrise Point */
+	LPC_KLK,	/* Kaby Lake */
+	LPC_CNPT,	/* Cannon Point */
 };
 
 static struct lpc_ich_info lpc_chipset_info[] = {
@@ -557,6 +575,18 @@ static struct lpc_ich_info lpc_chipset_info[] = {
 		.name = "Cougar Mountain SoC",
 		.iTCO_version = 3,
 	},
+	[LPC_SPT] = {
+		.name = "Sunrise Point",
+		.spi_type = INTEL_SPI_LPT,
+	},
+	[LPC_KLK] = {
+		.name = "Kaby Lake-H",
+		.spi_type = INTEL_SPI_LPT,
+	},
+	[LPC_CNPT] = {
+		.name = "Cannon Point",
+		.spi_type = INTEL_SPI_LPT,
+	},
 };
 
 /*
@@ -792,6 +822,21 @@ static const struct pci_device_id lpc_ich_ids[] = {
 	{ PCI_VDEVICE(INTEL, 0x9cc6), LPC_WPT_LP},
 	{ PCI_VDEVICE(INTEL, 0x9cc7), LPC_WPT_LP},
 	{ PCI_VDEVICE(INTEL, 0x9cc9), LPC_WPT_LP},
+	{ PCI_VDEVICE(INTEL, 0x9da4), LPC_CNPT},
+	{ PCI_VDEVICE(INTEL, 0xa143), LPC_SPT},
+	{ PCI_VDEVICE(INTEL, 0xa144), LPC_SPT},
+	{ PCI_VDEVICE(INTEL, 0xa145), LPC_SPT},
+	{ PCI_VDEVICE(INTEL, 0xa146), LPC_SPT},
+	{ PCI_VDEVICE(INTEL, 0xa147), LPC_SPT},
+	{ PCI_VDEVICE(INTEL, 0xa148), LPC_SPT},
+	{ PCI_VDEVICE(INTEL, 0xa149), LPC_SPT},
+	{ PCI_VDEVICE(INTEL, 0xa14a), LPC_SPT},
+	{ PCI_VDEVICE(INTEL, 0xa14d), LPC_SPT},
+	{ PCI_VDEVICE(INTEL, 0xa14e), LPC_SPT},
+	{ PCI_VDEVICE(INTEL, 0xa150), LPC_SPT},
+	{ PCI_VDEVICE(INTEL, 0xa152), LPC_KLK},
+	{ PCI_VDEVICE(INTEL, 0xa153), LPC_KLK},
+	{ PCI_VDEVICE(INTEL, 0xa154), LPC_KLK},
 	{ PCI_VDEVICE(INTEL, 0xa1c1), LPC_LEWISBURG},
 	{ PCI_VDEVICE(INTEL, 0xa1c2), LPC_LEWISBURG},
 	{ PCI_VDEVICE(INTEL, 0xa1c3), LPC_LEWISBURG},
@@ -1083,6 +1128,93 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
 	return ret;
 }
 
+#ifdef CONFIG_SECURITY
+static ssize_t bioswe_read(struct file *filp, char __user *buf,
+			   size_t count, loff_t *ppos)
+{
+	struct intel_spi_boardinfo *info = lpc_ich_spi_cell.platform_data;
+	char tmp[2];
+
+	sprintf(tmp, "%d\n", info->writeable ? 1 : 0);
+	return simple_read_from_buffer(buf, count, ppos, tmp, sizeof(tmp));
+}
+
+static const struct file_operations spi_bioswe_ops = {
+	.read  = bioswe_read,
+};
+
+static ssize_t ble_read(struct file *filp, char __user *buf,
+			size_t count, loff_t *ppos)
+{
+	struct intel_spi_boardinfo *info = lpc_ich_spi_cell.platform_data;
+	char tmp[2];
+
+	sprintf(tmp, "%d\n", info->ble ? 1 : 0);
+	return simple_read_from_buffer(buf, count, ppos, tmp, sizeof(tmp));
+}
+
+static const struct file_operations spi_ble_ops = {
+	.read  = ble_read,
+};
+
+static ssize_t smm_bwp_read(struct file *filp, char __user *buf,
+			    size_t count, loff_t *ppos)
+{
+	struct intel_spi_boardinfo *info = lpc_ich_spi_cell.platform_data;
+	char tmp[2];
+
+	sprintf(tmp, "%d\n", info->smm_bwp ? 1 : 0);
+	return simple_read_from_buffer(buf, count, ppos, tmp, sizeof(tmp));
+}
+
+static const struct file_operations spi_smm_bwp_ops = {
+	.read  = smm_bwp_read,
+};
+
+static int lpc_ich_init_securityfs(struct pci_dev *dev)
+{
+	struct lpc_ich_priv *priv = pci_get_drvdata(dev);
+
+	priv->spi_dir = securityfs_create_dir("spi", NULL);
+	if (IS_ERR(priv->spi_dir))
+		return -1;
+
+	priv->spi_bioswe =
+	    securityfs_create_file("bioswe",
+				   0600, priv->spi_dir, dev,
+				   &spi_bioswe_ops);
+	if (IS_ERR(priv->spi_bioswe))
+		goto out;
+	priv->spi_ble =
+	    securityfs_create_file("ble",
+				   0600, priv->spi_dir, dev,
+				   &spi_ble_ops);
+	if (IS_ERR(priv->spi_ble))
+		goto out;
+	priv->spi_smm_bwp =
+	    securityfs_create_file("smm_bwp",
+				   0600, priv->spi_dir, dev,
+				   &spi_smm_bwp_ops);
+	if (IS_ERR(priv->spi_smm_bwp))
+		goto out;
+	return 0;
+out:
+	securityfs_remove(priv->spi_ble);
+	securityfs_remove(priv->spi_bioswe);
+	securityfs_remove(priv->spi_dir);
+	return -1;
+}
+
+static void lpc_ich_uninit_securityfs(struct pci_dev *dev)
+{
+	struct lpc_ich_priv *priv = pci_get_drvdata(dev);
+	securityfs_remove(priv->spi_smm_bwp);
+	securityfs_remove(priv->spi_ble);
+	securityfs_remove(priv->spi_bioswe);
+	securityfs_remove(priv->spi_dir);
+}
+#endif
+
 static int lpc_ich_init_spi(struct pci_dev *dev)
 {
 	struct lpc_ich_priv *priv = pci_get_drvdata(dev);
@@ -1112,9 +1244,11 @@ static int lpc_ich_init_spi(struct pci_dev *dev)
 			res->start = spi_base + SPIBASE_LPT;
 			res->end = res->start + SPIBASE_LPT_SZ - 1;
 
-			pci_read_config_dword(dev, BCR, &bcr);
-			info->writeable = !!(bcr & BCR_WPD);
 		}
+		pci_read_config_dword(dev, BCR, &bcr);
+		info->writeable = !!(bcr & BCR_WPD);
+		info->ble = !!(bcr & BCR_BLE);
+		info->smm_bwp = !!(bcr & BCR_SMM_BWP);
 		break;
 
 	case INTEL_SPI_BXT: {
@@ -1136,6 +1270,8 @@ static int lpc_ich_init_spi(struct pci_dev *dev)
 
 			pci_bus_read_config_dword(bus, spi, BCR, &bcr);
 			info->writeable = !!(bcr & BCR_WPD);
+			info->ble = !!(bcr & BCR_BLE);
+			info->smm_bwp = !!(bcr & BCR_SMM_BWP);
 		}
 
 		pci_bus_write_config_byte(bus, p2sb, 0xe1, 0x1);
@@ -1146,8 +1282,9 @@ static int lpc_ich_init_spi(struct pci_dev *dev)
 		return -EINVAL;
 	}
 
-	if (!res->start)
-		return -ENODEV;
+// FIXME: no base address?
+//	if (!res->start)
+//		return -ENODEV;
 
 	lpc_ich_spi_cell.platform_data = info;
 	lpc_ich_spi_cell.pdata_size = sizeof(*info);
@@ -1201,8 +1338,13 @@ static int lpc_ich_probe(struct pci_dev *dev,
 
 	if (lpc_chipset_info[priv->chipset].spi_type) {
 		ret = lpc_ich_init_spi(dev);
-		if (!ret)
+		if (!ret) {
+#ifdef CONFIG_SECURITY
+			if (lpc_ich_init_securityfs (dev))
+				return -EINVAL;
+#endif
 			cell_added = true;
+		}
 	}
 
 	/*
@@ -1221,6 +1363,9 @@ static int lpc_ich_probe(struct pci_dev *dev,
 static void lpc_ich_remove(struct pci_dev *dev)
 {
 	mfd_remove_devices(&dev->dev);
+#ifdef CONFIG_SECURITY
+	lpc_ich_uninit_securityfs(dev);
+#endif
 	lpc_ich_restore_config_space(dev);
 }
 
diff --git a/include/linux/platform_data/intel-spi.h b/include/linux/platform_data/intel-spi.h
index 7f53a5c6f35e..83045b61ffc3 100644
--- a/include/linux/platform_data/intel-spi.h
+++ b/include/linux/platform_data/intel-spi.h
@@ -19,11 +19,15 @@ enum intel_spi_type {
 /**
  * struct intel_spi_boardinfo - Board specific data for Intel SPI driver
  * @type: Type which this controller is compatible with
- * @writeable: The chip is writeable
+ * @writeable: The chip is writeable, a.k.a. BIOSWE
+ * @ble: a SMM is raised when setting BIOSWE
+ * @smm_bwp: the BIOS region is non-writable unless all processors are in SMM
  */
 struct intel_spi_boardinfo {
 	enum intel_spi_type type;
 	bool writeable;
+	bool ble;
+	bool smm_bwp;
 };
 
 #endif /* INTEL_SPI_PDATA_H */
-- 
2.26.2


[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux