linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Platform integrity information in sysfs (version 9)
@ 2020-09-30 16:37 Daniel Gutson
  2020-09-30 16:37 ` [PATCH 1/2] " Daniel Gutson
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Daniel Gutson @ 2020-09-30 16:37 UTC (permalink / raw)
  To: Daniel Gutson, Derek Kiernan, Tudor Ambarus, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Mika Westerberg,
	Arnd Bergmann, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-kernel, Richard Hughes, Alex Bazhaniuk, linux-mtd

This patch serie adds a misc kernel module and extends the intel-spi drivers
to publish platform integrity data in the sys-fs.
Please check the comments in the following patches of this serie for further
details.

Daniel Gutson (2):
  Platform integrity information in sysfs (version 9)
  This patch exports the BIOS Write Enable (bioswe), BIOS Lock Enable
    (biosle), and the SMM BIOS Write Protect (SMM_BIOSWP) fields of the
    BIOS Control register using the platform-integrity misc kernel
    module. The idea is to keep adding more flags, not only from the BC
    but also from other registers in following versions.

 .../ABI/stable/sysfs-class-platform-integrity | 23 +++++
 MAINTAINERS                                   |  7 ++
 drivers/misc/Kconfig                          | 11 +++
 drivers/misc/Makefile                         |  1 +
 drivers/misc/platform-integrity.c             | 57 ++++++++++++
 drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
 .../mtd/spi-nor/controllers/intel-spi-pci.c   | 75 +++++++++++++++-
 .../spi-nor/controllers/intel-spi-platform.c  |  2 +-
 drivers/mtd/spi-nor/controllers/intel-spi.c   | 90 ++++++++++++++++++-
 drivers/mtd/spi-nor/controllers/intel-spi.h   |  9 +-
 include/linux/platform-integrity.h            | 19 ++++
 11 files changed, 288 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/ABI/stable/sysfs-class-platform-integrity
 create mode 100644 drivers/misc/platform-integrity.c
 create mode 100644 include/linux/platform-integrity.h

--
2.25.1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] Platform integrity information in sysfs (version 9)
  2020-09-30 16:37 [PATCH 0/2] Platform integrity information in sysfs (version 9) Daniel Gutson
@ 2020-09-30 16:37 ` Daniel Gutson
  2020-09-30 16:37 ` [PATCH 2/2] " Daniel Gutson
  2020-09-30 16:42 ` [PATCH 0/2] " Daniel Gutson
  2 siblings, 0 replies; 10+ messages in thread
From: Daniel Gutson @ 2020-09-30 16:37 UTC (permalink / raw)
  To: Daniel Gutson, Derek Kiernan, Tudor Ambarus, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Mika Westerberg,
	Arnd Bergmann, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-kernel, Richard Hughes, Alex Bazhaniuk, linux-mtd

This patch provides a driver and an API for exporting
information about the platform integrity
firmware configuration in the sysfs filesystem.

The goal is that the attributes are avilable to fwupd.

Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>

---
 .../ABI/stable/sysfs-class-platform-integrity | 23 ++++++++
 MAINTAINERS                                   |  7 +++
 drivers/misc/Kconfig                          | 11 ++++
 drivers/misc/Makefile                         |  1 +
 drivers/misc/platform-integrity.c             | 56 +++++++++++++++++++
 include/linux/platform-integrity.h            | 19 +++++++
 6 files changed, 117 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-class-platform-integrity
 create mode 100644 drivers/misc/platform-integrity.c
 create mode 100644 include/linux/platform-integrity.h

diff --git a/Documentation/ABI/stable/sysfs-class-platform-integrity b/Documentation/ABI/stable/sysfs-class-platform-integrity
new file mode 100644
index 000000000000..0978079bde50
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-class-platform-integrity
@@ -0,0 +1,23 @@
+What:		/sys/class/platform-integrity/intel-spi/bioswe
+Date:		September 2020
+KernelVersion:	5.10
+Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
+Description:	If the system firmware set BIOS Write Enable.
+		0: writes disabled, 1: writes enabled.
+Users:		https://github.com/fwupd/fwupd
+
+What:		/sys/class/platform-integrity/intel-spi/biosle
+Date:		September 2020
+KernelVersion:	5.10
+Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
+Description:	If the system firmware set BIOS Lock Enable.
+		0: SMM lock disabled, 1: SMM lock enabled.
+Users:		https://github.com/fwupd/fwupd
+
+What:		/sys/class/platform-integrity/intel-spi/smm_bioswp
+Date:		September 2020
+KernelVersion:	5.10
+Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
+Description:	If the system firmware set SMM BIOS Write Protect.
+		0: writes disabled unless in SMM, 1: writes enabled.
+Users:		https://github.com/fwupd/fwupd
diff --git a/MAINTAINERS b/MAINTAINERS
index d746519253c3..98bd26cd1adc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13774,6 +13774,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.yaml
 F:	drivers/iio/chemical/pms7003.c
 
+PLATFORM INTEGRITY DATA MODULE
+M:	Daniel Gutson <daniel.gutson@eclypsium.com>
+S:	Supported
+F:	Documentation/ABI/sysfs-class-platform-integrity
+F:	drivers/misc/platform-integrity.c
+F:	include/linux/platform-integrity.h
+
 PLDMFW LIBRARY
 M:	Jacob Keller <jacob.e.keller@intel.com>
 S:	Maintained
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index ce136d685d14..8602049bd0ad 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -456,6 +456,17 @@ config PVPANIC
 	  a paravirtualized device provided by QEMU; it lets a virtual machine
 	  (guest) communicate panic events to the host.
 
+config PLATFORM_INTEGRITY_DATA
+	bool "Platform integrity information in the sysfs"
+	depends on SYSFS
+	help
+	  This kernel module is a helper driver to provide information about
+	  platform integrity settings and configuration.
+	  This module is used by other device drivers -such as the intel-spi-
+	  to publish the information in /sys/class/platform-integrity which is
+	  consumed by software such as fwupd which can verify the platform
+	  has been configured in a secure way.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c7bd01ac6291..97ebb997fc47 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -57,3 +57,4 @@ obj-$(CONFIG_PVPANIC)   	+= pvpanic.o
 obj-$(CONFIG_HABANA_AI)		+= habanalabs/
 obj-$(CONFIG_UACCE)		+= uacce/
 obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
+obj-$(CONFIG_PLATFORM_INTEGRITY_DATA)	+= platform-integrity.o
diff --git a/drivers/misc/platform-integrity.c b/drivers/misc/platform-integrity.c
new file mode 100644
index 000000000000..e17d27850a3b
--- /dev/null
+++ b/drivers/misc/platform-integrity.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Platform integrity data kernel module
+ *
+ * Copyright (C) 2020 Daniel Gutson <daniel.gutson@eclypsium.com>
+ * Copyright (C) 2020 Eclypsium Inc.
+ */
+#include <linux/sysfs.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kdev_t.h>
+#include <linux/platform-integrity.h>
+
+static struct class platform_integrity_class = {
+	.name = "platform-integrity",
+	.owner = THIS_MODULE,
+};
+
+struct device *
+create_platform_integrity_device(struct device *parent, const char *name,
+				 const struct attribute_group **groups)
+{
+	return device_create_with_groups(&platform_integrity_class, parent,
+					 MKDEV(0, 0), groups, groups, "%s",
+					 name);
+}
+EXPORT_SYMBOL_GPL(create_platform_integrity_device);
+
+void destroy_platform_integrity_device(struct device *pi_device)
+{
+	device_remove_groups(pi_device,
+		(const struct attribute_group **)dev_get_drvdata(pi_device));
+	device_unregister(pi_device);
+}
+EXPORT_SYMBOL_GPL(destroy_platform_integrity_device);
+
+static int __init platform_integrity_init(void)
+{
+	int status;
+
+	status = class_register(&platform_integrity_class);
+	if (status < 0)
+		return status;
+
+	return 0;
+}
+
+static void __exit platform_integrity_exit(void)
+{
+	class_unregister(&platform_integrity_class);
+}
+
+module_init(platform_integrity_init);
+module_exit(platform_integrity_exit);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Daniel Gutson <daniel.gutson@eclypsium.com>");
diff --git a/include/linux/platform-integrity.h b/include/linux/platform-integrity.h
new file mode 100644
index 000000000000..56eb1a1190e8
--- /dev/null
+++ b/include/linux/platform-integrity.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Platform integrity data kernel module
+ *
+ * Copyright (C) 2020 Daniel Gutson <daniel.gutson@eclypsium.com>
+ * Copyright (C) 2020 Eclypsium Inc.
+ */
+#ifndef PLATFORM_INTEGRITY_H
+#define PLATFORM_INTEGRITY_H
+
+#include <linux/device.h>
+
+struct device *
+create_platform_integrity_device(struct device *parent, const char *name,
+				 const struct attribute_group **groups);
+
+extern void destroy_platform_integrity_device(struct device *pi_device);
+
+#endif /* PLATFORM_INTEGRITY_H */
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] Platform integrity information in sysfs (version 9)
  2020-09-30 16:37 [PATCH 0/2] Platform integrity information in sysfs (version 9) Daniel Gutson
  2020-09-30 16:37 ` [PATCH 1/2] " Daniel Gutson
@ 2020-09-30 16:37 ` Daniel Gutson
  2020-10-02 13:43   ` Greg Kroah-Hartman
  2020-10-04  4:01   ` Randy Dunlap
  2020-09-30 16:42 ` [PATCH 0/2] " Daniel Gutson
  2 siblings, 2 replies; 10+ messages in thread
From: Daniel Gutson @ 2020-09-30 16:37 UTC (permalink / raw)
  To: Daniel Gutson, Derek Kiernan, Tudor Ambarus, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Mika Westerberg,
	Arnd Bergmann, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-kernel, Richard Hughes, Alex Bazhaniuk, linux-mtd

This patch exports the BIOS Write Enable (bioswe), BIOS
Lock Enable (biosle), and the SMM BIOS Write Protect (SMM_BIOSWP) fields of
the BIOS Control register using the platform-integrity misc kernel module.
The idea is to keep adding more flags, not only from the BC but also from
other registers in following versions.

The goal is that the attributes are avilable to fwupd when SecureBoot
is turned on.

Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>

---
 drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
 .../mtd/spi-nor/controllers/intel-spi-pci.c   | 75 ++++++++++++++-
 .../spi-nor/controllers/intel-spi-platform.c  |  2 +-
 drivers/mtd/spi-nor/controllers/intel-spi.c   | 91 ++++++++++++++++++-
 drivers/mtd/spi-nor/controllers/intel-spi.h   |  9 +-
 5 files changed, 171 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
index 5c0e0ec2e6d1..e7eaef506fc2 100644
--- a/drivers/mtd/spi-nor/controllers/Kconfig
+++ b/drivers/mtd/spi-nor/controllers/Kconfig
@@ -29,6 +29,7 @@ config SPI_NXP_SPIFI
 
 config SPI_INTEL_SPI
 	tristate
+	depends on PLATFORM_INTEGRITY_DATA
 
 config SPI_INTEL_SPI_PCI
 	tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
index c72aa1ab71ad..644b1a6091dc 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
+++ b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
@@ -10,11 +10,19 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/platform-integrity.h>
 
 #include "intel-spi.h"
 
 #define BCR		0xdc
 #define BCR_WPD		BIT(0)
+#define BCR_BLE		BIT(1)
+#define BCR_SMM_BWP	BIT(5)
+
+struct cnl_spi_attr {
+	struct device_attribute dev_attr;
+	u32 mask;
+};
 
 static const struct intel_spi_boardinfo bxt_info = {
 	.type = INTEL_SPI_BXT,
@@ -24,6 +32,70 @@ static const struct intel_spi_boardinfo cnl_info = {
 	.type = INTEL_SPI_CNL,
 };
 
+#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
+static ssize_t intel_spi_cnl_spi_attr_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf, u32 mask)
+{
+	u32 bcr;
+
+	if (dev->parent == NULL)
+		return -EIO;
+
+	if (pci_read_config_dword(container_of(dev->parent, struct pci_dev, dev),
+				  BCR, &bcr) != PCIBIOS_SUCCESSFUL)
+		return -EIO;
+
+	return sprintf(buf, "%d\n", (int)!!(bcr & mask));
+}
+
+static ssize_t bioswe_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_WPD);
+}
+static DEVICE_ATTR_RO(bioswe);
+
+static ssize_t biosle_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_BLE);
+}
+static DEVICE_ATTR_RO(biosle);
+
+static ssize_t smm_bioswp_show(struct device *dev, struct device_attribute *attr,
+			       char *buf)
+{
+	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_SMM_BWP);
+}
+static DEVICE_ATTR_RO(smm_bioswp);
+
+static struct attribute *cnl_attrs[] = {
+	&dev_attr_bioswe.attr,
+	&dev_attr_biosle.attr,
+	&dev_attr_smm_bioswp.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(cnl);
+
+static struct device *
+register_local_platform_integrity_device(struct device *parent,
+					 enum intel_spi_type type)
+{
+	return create_platform_integrity_device(parent,	"intel-spi",
+		cnl_groups);
+}
+
+#else
+
+static struct device *
+register_local_platform_integrity_device(struct device *parent,
+					 enum intel_spi_type type)
+{
+	return NULL;
+}
+#endif /* CONFIG_PLATFORM_INTEGRITY_DATA */
+
 static int intel_spi_pci_probe(struct pci_dev *pdev,
 			       const struct pci_device_id *id)
 {
@@ -50,7 +122,8 @@ static int intel_spi_pci_probe(struct pci_dev *pdev,
 	}
 	info->writeable = !!(bcr & BCR_WPD);
 
-	ispi = intel_spi_probe(&pdev->dev, &pdev->resource[0], info);
+	ispi = intel_spi_probe(&pdev->dev, &pdev->resource[0], info,
+			       register_local_platform_integrity_device);
 	if (IS_ERR(ispi))
 		return PTR_ERR(ispi);
 
diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-platform.c b/drivers/mtd/spi-nor/controllers/intel-spi-platform.c
index f80f1086f928..5b1a9989426a 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi-platform.c
+++ b/drivers/mtd/spi-nor/controllers/intel-spi-platform.c
@@ -23,7 +23,7 @@ static int intel_spi_platform_probe(struct platform_device *pdev)
 		return -EINVAL;
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	ispi = intel_spi_probe(&pdev->dev, mem, info);
+	ispi = intel_spi_probe(&pdev->dev, mem, info, NULL);
 	if (IS_ERR(ispi))
 		return PTR_ERR(ispi);
 
diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c b/drivers/mtd/spi-nor/controllers/intel-spi.c
index b54a56a68100..ff0d2afc9bf4 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi.c
+++ b/drivers/mtd/spi-nor/controllers/intel-spi.c
@@ -16,6 +16,7 @@
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/spi-nor.h>
 #include <linux/platform_data/intel-spi.h>
+#include <linux/platform-integrity.h>
 
 #include "intel-spi.h"
 
@@ -95,6 +96,8 @@
 #define BYT_SSFSTS_CTL			0x90
 #define BYT_BCR				0xfc
 #define BYT_BCR_WPD			BIT(0)
+#define BYT_BCR_BLE			BIT(1)
+#define BYT_BCR_SMM_BWP			BIT(5)
 #define BYT_FREG_NUM			5
 #define BYT_PR_NUM			5
 
@@ -157,6 +160,7 @@ struct intel_spi {
 	bool erase_64k;
 	u8 atomic_preopcode;
 	u8 opcodes[8];
+	struct device *platform_integrity_device;
 };
 
 static bool writeable;
@@ -305,7 +309,69 @@ static int intel_spi_wait_sw_busy(struct intel_spi *ispi)
 				  INTEL_SPI_TIMEOUT * 1000);
 }
 
-static int intel_spi_init(struct intel_spi *ispi)
+#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
+static ssize_t byt_spi_attr_show(struct device *dev,
+				 struct device_attribute *attr, char *buf,
+				 u32 mask)
+{
+	u32 bcr;
+	struct intel_spi *ispi;
+
+	if (dev->parent == NULL)
+		return -EIO;
+
+	ispi = dev_get_drvdata(dev->parent);
+
+	bcr = readl(ispi->base + BYT_BCR);
+	return sprintf(buf, "%d\n", (int)!!(bcr & mask));
+}
+
+static ssize_t bioswe_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	return byt_spi_attr_show(dev, attr, buf, BYT_BCR_WPD);
+}
+static DEVICE_ATTR_RO(bioswe);
+
+static ssize_t biosle_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	return byt_spi_attr_show(dev, attr, buf, BYT_BCR_BLE);
+}
+static DEVICE_ATTR_RO(biosle);
+
+static ssize_t smm_bioswp_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return byt_spi_attr_show(dev, attr, buf, BYT_BCR_SMM_BWP);
+}
+static DEVICE_ATTR_RO(smm_bioswp);
+
+static struct attribute *byt_attrs[] = {
+	&dev_attr_bioswe.attr,
+	&dev_attr_biosle.attr,
+	&dev_attr_smm_bioswp.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(byt);
+
+static struct device *
+register_local_platform_integrity_device(struct device *parent,
+					 enum intel_spi_type type)
+{
+	switch (type) {
+	case INTEL_SPI_BYT:
+		return create_platform_integrity_device(parent, "intel-spi",
+							byt_groups);
+
+	default:
+		return NULL;
+	}
+}
+#endif /* CONFIG_PLATFORM_INTEGRITY_DATA */
+
+static int intel_spi_init(struct intel_spi *ispi,
+			  register_platform_integrity_t register_pi_cb)
 {
 	u32 opmenu0, opmenu1, lvscc, uvscc, val;
 	int i;
@@ -422,6 +488,15 @@ static int intel_spi_init(struct intel_spi *ispi)
 
 	intel_spi_dump_regs(ispi);
 
+#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
+	if (register_pi_cb == NULL)
+		register_pi_cb = register_local_platform_integrity_device;
+	ispi->platform_integrity_device = register_pi_cb(ispi->dev,
+							 ispi->info->type);
+	if (IS_ERR(ispi->platform_integrity_device))
+		ispi->platform_integrity_device = NULL;
+#endif
+
 	return 0;
 }
 
@@ -904,7 +979,8 @@ static const struct spi_nor_controller_ops intel_spi_controller_ops = {
 };
 
 struct intel_spi *intel_spi_probe(struct device *dev,
-	struct resource *mem, const struct intel_spi_boardinfo *info)
+	struct resource *mem, const struct intel_spi_boardinfo *info,
+	register_platform_integrity_t register_platform_integrity_cb)
 {
 	const struct spi_nor_hwcaps hwcaps = {
 		.mask = SNOR_HWCAPS_READ |
@@ -930,7 +1006,7 @@ struct intel_spi *intel_spi_probe(struct device *dev,
 	ispi->info = info;
 	ispi->writeable = info->writeable;
 
-	ret = intel_spi_init(ispi);
+	ret = intel_spi_init(ispi, register_platform_integrity_cb);
 	if (ret)
 		return ERR_PTR(ret);
 
@@ -960,6 +1036,15 @@ EXPORT_SYMBOL_GPL(intel_spi_probe);
 
 int intel_spi_remove(struct intel_spi *ispi)
 {
+#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
+	if (ispi->platform_integrity_device != NULL) {
+		destroy_platform_integrity_device(
+			ispi->platform_integrity_device);
+
+		ispi->platform_integrity_device = NULL;
+	}
+#endif /* CONFIG_PLATFORM_INTEGRITY_DATA */
+
 	return mtd_device_unregister(&ispi->nor.mtd);
 }
 EXPORT_SYMBOL_GPL(intel_spi_remove);
diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.h b/drivers/mtd/spi-nor/controllers/intel-spi.h
index e2f41b8827bf..5b00df0d4f19 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi.h
+++ b/drivers/mtd/spi-nor/controllers/intel-spi.h
@@ -14,8 +14,13 @@
 struct intel_spi;
 struct resource;
 
-struct intel_spi *intel_spi_probe(struct device *dev,
-	struct resource *mem, const struct intel_spi_boardinfo *info);
+typedef struct device *(*register_platform_integrity_t)(struct device *parent,
+			enum intel_spi_type type);
+
+struct intel_spi *intel_spi_probe(struct device *dev, struct resource *mem,
+				  const struct intel_spi_boardinfo *info,
+				  register_platform_integrity_t register_pi_cb);
+
 int intel_spi_remove(struct intel_spi *ispi);
 
 #endif /* INTEL_SPI_H */
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] Platform integrity information in sysfs (version 9)
  2020-09-30 16:37 [PATCH 0/2] Platform integrity information in sysfs (version 9) Daniel Gutson
  2020-09-30 16:37 ` [PATCH 1/2] " Daniel Gutson
  2020-09-30 16:37 ` [PATCH 2/2] " Daniel Gutson
@ 2020-09-30 16:42 ` Daniel Gutson
  2 siblings, 0 replies; 10+ messages in thread
From: Daniel Gutson @ 2020-09-30 16:42 UTC (permalink / raw)
  To: Daniel Gutson, Derek Kiernan, Tudor Ambarus, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Mika Westerberg,
	Arnd Bergmann, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-kernel, Richard Hughes, Alex Bazhaniuk, linux-mtd

Sorry, I forgot to restore
Tested-by: Richard Hughes <richard@hughsie.com>
which I removed for experimenting purposes with git-send-email.

On Wed, Sep 30, 2020 at 1:37 PM Daniel Gutson
<daniel.gutson@eclypsium.com> wrote:
>
> This patch serie adds a misc kernel module and extends the intel-spi drivers
> to publish platform integrity data in the sys-fs.
> Please check the comments in the following patches of this serie for further
> details.
>
> Daniel Gutson (2):
>   Platform integrity information in sysfs (version 9)
>   This patch exports the BIOS Write Enable (bioswe), BIOS Lock Enable
>     (biosle), and the SMM BIOS Write Protect (SMM_BIOSWP) fields of the
>     BIOS Control register using the platform-integrity misc kernel
>     module. The idea is to keep adding more flags, not only from the BC
>     but also from other registers in following versions.
>
>  .../ABI/stable/sysfs-class-platform-integrity | 23 +++++
>  MAINTAINERS                                   |  7 ++
>  drivers/misc/Kconfig                          | 11 +++
>  drivers/misc/Makefile                         |  1 +
>  drivers/misc/platform-integrity.c             | 57 ++++++++++++
>  drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
>  .../mtd/spi-nor/controllers/intel-spi-pci.c   | 75 +++++++++++++++-
>  .../spi-nor/controllers/intel-spi-platform.c  |  2 +-
>  drivers/mtd/spi-nor/controllers/intel-spi.c   | 90 ++++++++++++++++++-
>  drivers/mtd/spi-nor/controllers/intel-spi.h   |  9 +-
>  include/linux/platform-integrity.h            | 19 ++++
>  11 files changed, 288 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/ABI/stable/sysfs-class-platform-integrity
>  create mode 100644 drivers/misc/platform-integrity.c
>  create mode 100644 include/linux/platform-integrity.h
>
> --
> 2.25.1



-- 


Daniel Gutson
Engineering Director
Eclypsium, Inc.


Below The Surface: Get the latest threat research and insights on
firmware and supply chain threats from the research team at Eclypsium.
https://eclypsium.com/research/#threatreport

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Platform integrity information in sysfs (version 9)
  2020-09-30 16:37 ` [PATCH 2/2] " Daniel Gutson
@ 2020-10-02 13:43   ` Greg Kroah-Hartman
  2020-10-21 19:55     ` Daniel Gutson
  2020-10-04  4:01   ` Randy Dunlap
  1 sibling, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2020-10-02 13:43 UTC (permalink / raw)
  To: Daniel Gutson
  Cc: Vignesh Raghavendra, Arnd Bergmann, Tudor Ambarus,
	Mauro Carvalho Chehab, Richard Weinberger, Richard Hughes,
	linux-kernel, linux-mtd, Miquel Raynal, Derek Kiernan,
	Mika Westerberg, Alex Bazhaniuk

On Wed, Sep 30, 2020 at 01:37:14PM -0300, Daniel Gutson wrote:
> This patch exports the BIOS Write Enable (bioswe), BIOS
> Lock Enable (biosle), and the SMM BIOS Write Protect (SMM_BIOSWP) fields of
> the BIOS Control register using the platform-integrity misc kernel module.
> The idea is to keep adding more flags, not only from the BC but also from
> other registers in following versions.
> 
> The goal is that the attributes are avilable to fwupd when SecureBoot
> is turned on.
> 
> Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>

The subject line doesn't match what this patch does, please fix that up.

But there are more core issues in this patch:

> 
> ---
>  drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
>  .../mtd/spi-nor/controllers/intel-spi-pci.c   | 75 ++++++++++++++-
>  .../spi-nor/controllers/intel-spi-platform.c  |  2 +-
>  drivers/mtd/spi-nor/controllers/intel-spi.c   | 91 ++++++++++++++++++-
>  drivers/mtd/spi-nor/controllers/intel-spi.h   |  9 +-
>  5 files changed, 171 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
> index 5c0e0ec2e6d1..e7eaef506fc2 100644
> --- a/drivers/mtd/spi-nor/controllers/Kconfig
> +++ b/drivers/mtd/spi-nor/controllers/Kconfig
> @@ -29,6 +29,7 @@ config SPI_NXP_SPIFI
>  
>  config SPI_INTEL_SPI
>  	tristate
> +	depends on PLATFORM_INTEGRITY_DATA
>  
>  config SPI_INTEL_SPI_PCI
>  	tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
> diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> index c72aa1ab71ad..644b1a6091dc 100644
> --- a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> +++ b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> @@ -10,11 +10,19 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
> +#include <linux/platform-integrity.h>
>  
>  #include "intel-spi.h"
>  
>  #define BCR		0xdc
>  #define BCR_WPD		BIT(0)
> +#define BCR_BLE		BIT(1)
> +#define BCR_SMM_BWP	BIT(5)
> +
> +struct cnl_spi_attr {
> +	struct device_attribute dev_attr;
> +	u32 mask;
> +};
>  
>  static const struct intel_spi_boardinfo bxt_info = {
>  	.type = INTEL_SPI_BXT,
> @@ -24,6 +32,70 @@ static const struct intel_spi_boardinfo cnl_info = {
>  	.type = INTEL_SPI_CNL,
>  };
>  
> +#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
> +static ssize_t intel_spi_cnl_spi_attr_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf, u32 mask)
> +{
> +	u32 bcr;
> +
> +	if (dev->parent == NULL)
> +		return -EIO;
> +
> +	if (pci_read_config_dword(container_of(dev->parent, struct pci_dev, dev),
> +				  BCR, &bcr) != PCIBIOS_SUCCESSFUL)
> +		return -EIO;
> +
> +	return sprintf(buf, "%d\n", (int)!!(bcr & mask));
> +}
> +
> +static ssize_t bioswe_show(struct device *dev, struct device_attribute *attr,
> +			   char *buf)
> +{
> +	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_WPD);
> +}
> +static DEVICE_ATTR_RO(bioswe);
> +
> +static ssize_t biosle_show(struct device *dev, struct device_attribute *attr,
> +			   char *buf)
> +{
> +	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_BLE);
> +}
> +static DEVICE_ATTR_RO(biosle);
> +
> +static ssize_t smm_bioswp_show(struct device *dev, struct device_attribute *attr,
> +			       char *buf)
> +{
> +	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_SMM_BWP);
> +}
> +static DEVICE_ATTR_RO(smm_bioswp);
> +
> +static struct attribute *cnl_attrs[] = {
> +	&dev_attr_bioswe.attr,
> +	&dev_attr_biosle.attr,
> +	&dev_attr_smm_bioswp.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(cnl);

If you are forcing the driver to create the groups, then you are forcing
us to audit each driver and verify that the files are the same name and
such.  Put the files in the "common" code please, and if you really need
a way to get the data out, make that a callback or something.

Also, this name "platform integrity" is really really generic, while in
reality you are describing something very specific.  Are you sure you
want that?

thanks,

greg k-h

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Platform integrity information in sysfs (version 9)
  2020-09-30 16:37 ` [PATCH 2/2] " Daniel Gutson
  2020-10-02 13:43   ` Greg Kroah-Hartman
@ 2020-10-04  4:01   ` Randy Dunlap
  2020-10-22 12:08     ` Daniel Gutson
  1 sibling, 1 reply; 10+ messages in thread
From: Randy Dunlap @ 2020-10-04  4:01 UTC (permalink / raw)
  To: Daniel Gutson, Derek Kiernan, Tudor Ambarus, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Mika Westerberg,
	Arnd Bergmann, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-kernel, Richard Hughes, Alex Bazhaniuk, linux-mtd

On 9/30/20 9:37 AM, Daniel Gutson wrote:
> diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
> index 5c0e0ec2e6d1..e7eaef506fc2 100644
> --- a/drivers/mtd/spi-nor/controllers/Kconfig
> +++ b/drivers/mtd/spi-nor/controllers/Kconfig
> @@ -29,6 +29,7 @@ config SPI_NXP_SPIFI
>  
>  config SPI_INTEL_SPI
>  	tristate
> +	depends on PLATFORM_INTEGRITY_DATA

So SPI_INTEL_SPI_PCI selects SPI_INTEL_SPI:

config SPI_INTEL_SPI_PCI
	tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
	depends on X86 && PCI
	select SPI_INTEL_SPI

without checking that PLATFORM_INTEGRITY_DATA is set/enabled.

"select" does not follow any kconfig dependency chains, so when
PLATFORM_INTEGRITY_DATA is not enabled, this should be causing
a kconfig warning, which is not OK.


-- 
~Randy


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Platform integrity information in sysfs (version 9)
  2020-10-02 13:43   ` Greg Kroah-Hartman
@ 2020-10-21 19:55     ` Daniel Gutson
  2020-11-10 14:07       ` Daniel Gutson
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Gutson @ 2020-10-21 19:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vignesh Raghavendra, Arnd Bergmann, Tudor Ambarus,
	Mauro Carvalho Chehab, Richard Weinberger, Richard Hughes,
	linux-kernel, linux-mtd, Miquel Raynal, Derek Kiernan,
	Mika Westerberg, Alex Bazhaniuk

On Fri, Oct 2, 2020 at 10:43 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Sep 30, 2020 at 01:37:14PM -0300, Daniel Gutson wrote:
> > This patch exports the BIOS Write Enable (bioswe), BIOS
> > Lock Enable (biosle), and the SMM BIOS Write Protect (SMM_BIOSWP) fields of
> > the BIOS Control register using the platform-integrity misc kernel module.
> > The idea is to keep adding more flags, not only from the BC but also from
> > other registers in following versions.
> >
> > The goal is that the attributes are avilable to fwupd when SecureBoot
> > is turned on.
> >
> > Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>
>
> The subject line doesn't match what this patch does, please fix that up.
>
> But there are more core issues in this patch:
>
> >
> > ---
> >  drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
> >  .../mtd/spi-nor/controllers/intel-spi-pci.c   | 75 ++++++++++++++-
> >  .../spi-nor/controllers/intel-spi-platform.c  |  2 +-
> >  drivers/mtd/spi-nor/controllers/intel-spi.c   | 91 ++++++++++++++++++-
> >  drivers/mtd/spi-nor/controllers/intel-spi.h   |  9 +-
> >  5 files changed, 171 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
> > index 5c0e0ec2e6d1..e7eaef506fc2 100644
> > --- a/drivers/mtd/spi-nor/controllers/Kconfig
> > +++ b/drivers/mtd/spi-nor/controllers/Kconfig
> > @@ -29,6 +29,7 @@ config SPI_NXP_SPIFI
> >
> >  config SPI_INTEL_SPI
> >       tristate
> > +     depends on PLATFORM_INTEGRITY_DATA
> >
> >  config SPI_INTEL_SPI_PCI
> >       tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
> > diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> > index c72aa1ab71ad..644b1a6091dc 100644
> > --- a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> > +++ b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> > @@ -10,11 +10,19 @@
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/pci.h>
> > +#include <linux/platform-integrity.h>
> >
> >  #include "intel-spi.h"
> >
> >  #define BCR          0xdc
> >  #define BCR_WPD              BIT(0)
> > +#define BCR_BLE              BIT(1)
> > +#define BCR_SMM_BWP  BIT(5)
> > +
> > +struct cnl_spi_attr {
> > +     struct device_attribute dev_attr;
> > +     u32 mask;
> > +};
> >
> >  static const struct intel_spi_boardinfo bxt_info = {
> >       .type = INTEL_SPI_BXT,
> > @@ -24,6 +32,70 @@ static const struct intel_spi_boardinfo cnl_info = {
> >       .type = INTEL_SPI_CNL,
> >  };
> >
> > +#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
> > +static ssize_t intel_spi_cnl_spi_attr_show(struct device *dev,
> > +                                        struct device_attribute *attr,
> > +                                        char *buf, u32 mask)
> > +{
> > +     u32 bcr;
> > +
> > +     if (dev->parent == NULL)
> > +             return -EIO;
> > +
> > +     if (pci_read_config_dword(container_of(dev->parent, struct pci_dev, dev),
> > +                               BCR, &bcr) != PCIBIOS_SUCCESSFUL)
> > +             return -EIO;
> > +
> > +     return sprintf(buf, "%d\n", (int)!!(bcr & mask));
> > +}
> > +
> > +static ssize_t bioswe_show(struct device *dev, struct device_attribute *attr,
> > +                        char *buf)
> > +{
> > +     return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_WPD);
> > +}
> > +static DEVICE_ATTR_RO(bioswe);
> > +
> > +static ssize_t biosle_show(struct device *dev, struct device_attribute *attr,
> > +                        char *buf)
> > +{
> > +     return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_BLE);
> > +}
> > +static DEVICE_ATTR_RO(biosle);
> > +
> > +static ssize_t smm_bioswp_show(struct device *dev, struct device_attribute *attr,
> > +                            char *buf)
> > +{
> > +     return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_SMM_BWP);
> > +}
> > +static DEVICE_ATTR_RO(smm_bioswp);
> > +
> > +static struct attribute *cnl_attrs[] = {
> > +     &dev_attr_bioswe.attr,
> > +     &dev_attr_biosle.attr,
> > +     &dev_attr_smm_bioswp.attr,
> > +     NULL
> > +};
> > +ATTRIBUTE_GROUPS(cnl);
>
> If you are forcing the driver to create the groups, then you are forcing
> us to audit each driver and verify that the files are the same name and
> such.  Put the files in the "common" code please, and if you really need
> a way to get the data out, make that a callback or something.

If I understand you correctly, you are asking the opposite that Arnd
asked me in a
previous patch version: he told me no new callbacks, just use the
device attribute API.
However I'm not sure I understand your proposal, do you mean to let
the device attr
stay in the driver file, and do the group inside the common part? Therefore,
to pass a dev attributes array to the common part?
If not, could you please explain your proposal again?

>
> Also, this name "platform integrity" is really really generic, while in
> reality you are describing something very specific.  Are you sure you
> want that?
>
> thanks,
>
> greg k-h



-- 


Daniel Gutson
Engineering Director
Eclypsium, Inc.


Below The Surface: Get the latest threat research and insights on
firmware and supply chain threats from the research team at Eclypsium.
https://eclypsium.com/research/#threatreport

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Platform integrity information in sysfs (version 9)
  2020-10-04  4:01   ` Randy Dunlap
@ 2020-10-22 12:08     ` Daniel Gutson
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Gutson @ 2020-10-22 12:08 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Vignesh Raghavendra, Arnd Bergmann, Tudor Ambarus,
	Mauro Carvalho Chehab, Richard Weinberger, Richard Hughes,
	Greg Kroah-Hartman, linux-kernel, linux-mtd, Miquel Raynal,
	Derek Kiernan, Mika Westerberg, Alex Bazhaniuk

On Sun, Oct 4, 2020 at 1:01 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 9/30/20 9:37 AM, Daniel Gutson wrote:
> > diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
> > index 5c0e0ec2e6d1..e7eaef506fc2 100644
> > --- a/drivers/mtd/spi-nor/controllers/Kconfig
> > +++ b/drivers/mtd/spi-nor/controllers/Kconfig
> > @@ -29,6 +29,7 @@ config SPI_NXP_SPIFI
> >
> >  config SPI_INTEL_SPI
> >       tristate
> > +     depends on PLATFORM_INTEGRITY_DATA
>
> So SPI_INTEL_SPI_PCI selects SPI_INTEL_SPI:
>
> config SPI_INTEL_SPI_PCI
>         tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
>         depends on X86 && PCI
>         select SPI_INTEL_SPI
>
> without checking that PLATFORM_INTEGRITY_DATA is set/enabled.
>
> "select" does not follow any kconfig dependency chains, so when
> PLATFORM_INTEGRITY_DATA is not enabled, this should be causing
> a kconfig warning, which is not OK.

Since now SPI_INTEL_SPI_PCI can be enabled without PLATFORM_INTEGRITY_DATA
(thanks to the #ifdefs), I think I'll just remove the 'depends' and
will leave this as it was.

>
>
> --
> ~Randy
>


-- 


Daniel Gutson
Engineering Director
Eclypsium, Inc.


Below The Surface: Get the latest threat research and insights on
firmware and supply chain threats from the research team at Eclypsium.
https://eclypsium.com/research/#threatreport

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] Platform integrity information in sysfs (version 9)
  2020-10-21 19:55     ` Daniel Gutson
@ 2020-11-10 14:07       ` Daniel Gutson
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Gutson @ 2020-11-10 14:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vignesh Raghavendra, Arnd Bergmann, Tudor Ambarus,
	Mauro Carvalho Chehab, Richard Weinberger, Richard Hughes,
	linux-kernel, linux-mtd, Miquel Raynal, Derek Kiernan,
	Mika Westerberg, Alex Bazhaniuk, Yoav Yaari

On Wed, Oct 21, 2020 at 4:55 PM Daniel Gutson <daniel@eclypsium.com> wrote:
>
> On Fri, Oct 2, 2020 at 10:43 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Sep 30, 2020 at 01:37:14PM -0300, Daniel Gutson wrote:
> > > This patch exports the BIOS Write Enable (bioswe), BIOS
> > > Lock Enable (biosle), and the SMM BIOS Write Protect (SMM_BIOSWP) fields of
> > > the BIOS Control register using the platform-integrity misc kernel module.
> > > The idea is to keep adding more flags, not only from the BC but also from
> > > other registers in following versions.
> > >
> > > The goal is that the attributes are avilable to fwupd when SecureBoot
> > > is turned on.
> > >
> > > Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>
> >
> > The subject line doesn't match what this patch does, please fix that up.
> >
> > But there are more core issues in this patch:
> >
> > >
> > > ---
> > >  drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
> > >  .../mtd/spi-nor/controllers/intel-spi-pci.c   | 75 ++++++++++++++-
> > >  .../spi-nor/controllers/intel-spi-platform.c  |  2 +-
> > >  drivers/mtd/spi-nor/controllers/intel-spi.c   | 91 ++++++++++++++++++-
> > >  drivers/mtd/spi-nor/controllers/intel-spi.h   |  9 +-
> > >  5 files changed, 171 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
> > > index 5c0e0ec2e6d1..e7eaef506fc2 100644
> > > --- a/drivers/mtd/spi-nor/controllers/Kconfig
> > > +++ b/drivers/mtd/spi-nor/controllers/Kconfig
> > > @@ -29,6 +29,7 @@ config SPI_NXP_SPIFI
> > >
> > >  config SPI_INTEL_SPI
> > >       tristate
> > > +     depends on PLATFORM_INTEGRITY_DATA
> > >
> > >  config SPI_INTEL_SPI_PCI
> > >       tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
> > > diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> > > index c72aa1ab71ad..644b1a6091dc 100644
> > > --- a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> > > +++ b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> > > @@ -10,11 +10,19 @@
> > >  #include <linux/kernel.h>
> > >  #include <linux/module.h>
> > >  #include <linux/pci.h>
> > > +#include <linux/platform-integrity.h>
> > >
> > >  #include "intel-spi.h"
> > >
> > >  #define BCR          0xdc
> > >  #define BCR_WPD              BIT(0)
> > > +#define BCR_BLE              BIT(1)
> > > +#define BCR_SMM_BWP  BIT(5)
> > > +
> > > +struct cnl_spi_attr {
> > > +     struct device_attribute dev_attr;
> > > +     u32 mask;
> > > +};
> > >
> > >  static const struct intel_spi_boardinfo bxt_info = {
> > >       .type = INTEL_SPI_BXT,
> > > @@ -24,6 +32,70 @@ static const struct intel_spi_boardinfo cnl_info = {
> > >       .type = INTEL_SPI_CNL,
> > >  };
> > >
> > > +#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
> > > +static ssize_t intel_spi_cnl_spi_attr_show(struct device *dev,
> > > +                                        struct device_attribute *attr,
> > > +                                        char *buf, u32 mask)
> > > +{
> > > +     u32 bcr;
> > > +
> > > +     if (dev->parent == NULL)
> > > +             return -EIO;
> > > +
> > > +     if (pci_read_config_dword(container_of(dev->parent, struct pci_dev, dev),
> > > +                               BCR, &bcr) != PCIBIOS_SUCCESSFUL)
> > > +             return -EIO;
> > > +
> > > +     return sprintf(buf, "%d\n", (int)!!(bcr & mask));
> > > +}
> > > +
> > > +static ssize_t bioswe_show(struct device *dev, struct device_attribute *attr,
> > > +                        char *buf)
> > > +{
> > > +     return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_WPD);
> > > +}
> > > +static DEVICE_ATTR_RO(bioswe);
> > > +
> > > +static ssize_t biosle_show(struct device *dev, struct device_attribute *attr,
> > > +                        char *buf)
> > > +{
> > > +     return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_BLE);
> > > +}
> > > +static DEVICE_ATTR_RO(biosle);
> > > +
> > > +static ssize_t smm_bioswp_show(struct device *dev, struct device_attribute *attr,
> > > +                            char *buf)
> > > +{
> > > +     return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_SMM_BWP);
> > > +}
> > > +static DEVICE_ATTR_RO(smm_bioswp);
> > > +
> > > +static struct attribute *cnl_attrs[] = {
> > > +     &dev_attr_bioswe.attr,
> > > +     &dev_attr_biosle.attr,
> > > +     &dev_attr_smm_bioswp.attr,
> > > +     NULL
> > > +};
> > > +ATTRIBUTE_GROUPS(cnl);
> >
> > If you are forcing the driver to create the groups, then you are forcing
> > us to audit each driver and verify that the files are the same name and
> > such.  Put the files in the "common" code please, and if you really need
> > a way to get the data out, make that a callback or something.
>
> If I understand you correctly, you are asking the opposite that Arnd
> asked me in a
> previous patch version: he told me no new callbacks, just use the
> device attribute API.
> However I'm not sure I understand your proposal, do you mean to let
> the device attr
> stay in the driver file, and do the group inside the common part? Therefore,
> to pass a dev attributes array to the common part?
> If not, could you please explain your proposal again?


ping please.

>
> >
> > Also, this name "platform integrity" is really really generic, while in
> > reality you are describing something very specific.  Are you sure you
> > want that?
> >
> > thanks,
> >
> > greg k-h
>
>
>
> --
>
>
> Daniel Gutson
> Engineering Director
> Eclypsium, Inc.
>
>
> Below The Surface: Get the latest threat research and insights on
> firmware and supply chain threats from the research team at Eclypsium.
> https://eclypsium.com/research/#threatreport



-- 


Daniel Gutson
Engineering Director
Eclypsium, Inc.


Below The Surface: Get the latest threat research and insights on
firmware and supply chain threats from the research team at Eclypsium.
https://eclypsium.com/research/#threatreport

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] Platform integrity information in sysfs (version 9)
@ 2020-09-30 13:51 Daniel Gutson
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Gutson @ 2020-09-30 13:51 UTC (permalink / raw)
  To: Daniel Gutson, Derek Kiernan, Tudor Ambarus, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Mika Westerberg,
	Arnd Bergmann, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-kernel, Richard Hughes, Alex Bazhaniuk, linux-mtd
  Cc: Richard Hughes

This patch exports the BIOS Write Enable (bioswe), BIOS
Lock Enable (biosle), and the SMM BIOS Write Protect (SMM_BIOSWP) fields of
the BIOS Control register using the platform-integrity misc kernel module.
The idea is to keep adding more flags, not only from the BC but also from
other registers in following versions.

The goal is that the attributes are avilable to fwupd when SecureBoot
is turned on.

Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>
Tested-by: Richard Hughes <richard@hughsie.com>
---
 drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
 .../mtd/spi-nor/controllers/intel-spi-pci.c   | 75 ++++++++++++++-
 .../spi-nor/controllers/intel-spi-platform.c  |  2 +-
 drivers/mtd/spi-nor/controllers/intel-spi.c   | 91 ++++++++++++++++++-
 drivers/mtd/spi-nor/controllers/intel-spi.h   |  9 +-
 5 files changed, 171 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
index 5c0e0ec2e6d1..e7eaef506fc2 100644
--- a/drivers/mtd/spi-nor/controllers/Kconfig
+++ b/drivers/mtd/spi-nor/controllers/Kconfig
@@ -29,6 +29,7 @@ config SPI_NXP_SPIFI
 
 config SPI_INTEL_SPI
 	tristate
+	depends on PLATFORM_INTEGRITY_DATA
 
 config SPI_INTEL_SPI_PCI
 	tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
index c72aa1ab71ad..644b1a6091dc 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
+++ b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
@@ -10,11 +10,19 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/platform-integrity.h>
 
 #include "intel-spi.h"
 
 #define BCR		0xdc
 #define BCR_WPD		BIT(0)
+#define BCR_BLE		BIT(1)
+#define BCR_SMM_BWP	BIT(5)
+
+struct cnl_spi_attr {
+	struct device_attribute dev_attr;
+	u32 mask;
+};
 
 static const struct intel_spi_boardinfo bxt_info = {
 	.type = INTEL_SPI_BXT,
@@ -24,6 +32,70 @@ static const struct intel_spi_boardinfo cnl_info = {
 	.type = INTEL_SPI_CNL,
 };
 
+#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
+static ssize_t intel_spi_cnl_spi_attr_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf, u32 mask)
+{
+	u32 bcr;
+
+	if (dev->parent == NULL)
+		return -EIO;
+
+	if (pci_read_config_dword(container_of(dev->parent, struct pci_dev, dev),
+				  BCR, &bcr) != PCIBIOS_SUCCESSFUL)
+		return -EIO;
+
+	return sprintf(buf, "%d\n", (int)!!(bcr & mask));
+}
+
+static ssize_t bioswe_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_WPD);
+}
+static DEVICE_ATTR_RO(bioswe);
+
+static ssize_t biosle_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_BLE);
+}
+static DEVICE_ATTR_RO(biosle);
+
+static ssize_t smm_bioswp_show(struct device *dev, struct device_attribute *attr,
+			       char *buf)
+{
+	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_SMM_BWP);
+}
+static DEVICE_ATTR_RO(smm_bioswp);
+
+static struct attribute *cnl_attrs[] = {
+	&dev_attr_bioswe.attr,
+	&dev_attr_biosle.attr,
+	&dev_attr_smm_bioswp.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(cnl);
+
+static struct device *
+register_local_platform_integrity_device(struct device *parent,
+					 enum intel_spi_type type)
+{
+	return create_platform_integrity_device(parent,	"intel-spi",
+		cnl_groups);
+}
+
+#else
+
+static struct device *
+register_local_platform_integrity_device(struct device *parent,
+					 enum intel_spi_type type)
+{
+	return NULL;
+}
+#endif /* CONFIG_PLATFORM_INTEGRITY_DATA */
+
 static int intel_spi_pci_probe(struct pci_dev *pdev,
 			       const struct pci_device_id *id)
 {
@@ -50,7 +122,8 @@ static int intel_spi_pci_probe(struct pci_dev *pdev,
 	}
 	info->writeable = !!(bcr & BCR_WPD);
 
-	ispi = intel_spi_probe(&pdev->dev, &pdev->resource[0], info);
+	ispi = intel_spi_probe(&pdev->dev, &pdev->resource[0], info,
+			       register_local_platform_integrity_device);
 	if (IS_ERR(ispi))
 		return PTR_ERR(ispi);
 
diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-platform.c b/drivers/mtd/spi-nor/controllers/intel-spi-platform.c
index f80f1086f928..5b1a9989426a 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi-platform.c
+++ b/drivers/mtd/spi-nor/controllers/intel-spi-platform.c
@@ -23,7 +23,7 @@ static int intel_spi_platform_probe(struct platform_device *pdev)
 		return -EINVAL;
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	ispi = intel_spi_probe(&pdev->dev, mem, info);
+	ispi = intel_spi_probe(&pdev->dev, mem, info, NULL);
 	if (IS_ERR(ispi))
 		return PTR_ERR(ispi);
 
diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c b/drivers/mtd/spi-nor/controllers/intel-spi.c
index b54a56a68100..ff0d2afc9bf4 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi.c
+++ b/drivers/mtd/spi-nor/controllers/intel-spi.c
@@ -16,6 +16,7 @@
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/spi-nor.h>
 #include <linux/platform_data/intel-spi.h>
+#include <linux/platform-integrity.h>
 
 #include "intel-spi.h"
 
@@ -95,6 +96,8 @@
 #define BYT_SSFSTS_CTL			0x90
 #define BYT_BCR				0xfc
 #define BYT_BCR_WPD			BIT(0)
+#define BYT_BCR_BLE			BIT(1)
+#define BYT_BCR_SMM_BWP			BIT(5)
 #define BYT_FREG_NUM			5
 #define BYT_PR_NUM			5
 
@@ -157,6 +160,7 @@ struct intel_spi {
 	bool erase_64k;
 	u8 atomic_preopcode;
 	u8 opcodes[8];
+	struct device *platform_integrity_device;
 };
 
 static bool writeable;
@@ -305,7 +309,69 @@ static int intel_spi_wait_sw_busy(struct intel_spi *ispi)
 				  INTEL_SPI_TIMEOUT * 1000);
 }
 
-static int intel_spi_init(struct intel_spi *ispi)
+#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
+static ssize_t byt_spi_attr_show(struct device *dev,
+				 struct device_attribute *attr, char *buf,
+				 u32 mask)
+{
+	u32 bcr;
+	struct intel_spi *ispi;
+
+	if (dev->parent == NULL)
+		return -EIO;
+
+	ispi = dev_get_drvdata(dev->parent);
+
+	bcr = readl(ispi->base + BYT_BCR);
+	return sprintf(buf, "%d\n", (int)!!(bcr & mask));
+}
+
+static ssize_t bioswe_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	return byt_spi_attr_show(dev, attr, buf, BYT_BCR_WPD);
+}
+static DEVICE_ATTR_RO(bioswe);
+
+static ssize_t biosle_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	return byt_spi_attr_show(dev, attr, buf, BYT_BCR_BLE);
+}
+static DEVICE_ATTR_RO(biosle);
+
+static ssize_t smm_bioswp_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return byt_spi_attr_show(dev, attr, buf, BYT_BCR_SMM_BWP);
+}
+static DEVICE_ATTR_RO(smm_bioswp);
+
+static struct attribute *byt_attrs[] = {
+	&dev_attr_bioswe.attr,
+	&dev_attr_biosle.attr,
+	&dev_attr_smm_bioswp.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(byt);
+
+static struct device *
+register_local_platform_integrity_device(struct device *parent,
+					 enum intel_spi_type type)
+{
+	switch (type) {
+	case INTEL_SPI_BYT:
+		return create_platform_integrity_device(parent, "intel-spi",
+							byt_groups);
+
+	default:
+		return NULL;
+	}
+}
+#endif /* CONFIG_PLATFORM_INTEGRITY_DATA */
+
+static int intel_spi_init(struct intel_spi *ispi,
+			  register_platform_integrity_t register_pi_cb)
 {
 	u32 opmenu0, opmenu1, lvscc, uvscc, val;
 	int i;
@@ -422,6 +488,15 @@ static int intel_spi_init(struct intel_spi *ispi)
 
 	intel_spi_dump_regs(ispi);
 
+#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
+	if (register_pi_cb == NULL)
+		register_pi_cb = register_local_platform_integrity_device;
+	ispi->platform_integrity_device = register_pi_cb(ispi->dev,
+							 ispi->info->type);
+	if (IS_ERR(ispi->platform_integrity_device))
+		ispi->platform_integrity_device = NULL;
+#endif
+
 	return 0;
 }
 
@@ -904,7 +979,8 @@ static const struct spi_nor_controller_ops intel_spi_controller_ops = {
 };
 
 struct intel_spi *intel_spi_probe(struct device *dev,
-	struct resource *mem, const struct intel_spi_boardinfo *info)
+	struct resource *mem, const struct intel_spi_boardinfo *info,
+	register_platform_integrity_t register_platform_integrity_cb)
 {
 	const struct spi_nor_hwcaps hwcaps = {
 		.mask = SNOR_HWCAPS_READ |
@@ -930,7 +1006,7 @@ struct intel_spi *intel_spi_probe(struct device *dev,
 	ispi->info = info;
 	ispi->writeable = info->writeable;
 
-	ret = intel_spi_init(ispi);
+	ret = intel_spi_init(ispi, register_platform_integrity_cb);
 	if (ret)
 		return ERR_PTR(ret);
 
@@ -960,6 +1036,15 @@ EXPORT_SYMBOL_GPL(intel_spi_probe);
 
 int intel_spi_remove(struct intel_spi *ispi)
 {
+#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
+	if (ispi->platform_integrity_device != NULL) {
+		destroy_platform_integrity_device(
+			ispi->platform_integrity_device);
+
+		ispi->platform_integrity_device = NULL;
+	}
+#endif /* CONFIG_PLATFORM_INTEGRITY_DATA */
+
 	return mtd_device_unregister(&ispi->nor.mtd);
 }
 EXPORT_SYMBOL_GPL(intel_spi_remove);
diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.h b/drivers/mtd/spi-nor/controllers/intel-spi.h
index e2f41b8827bf..5b00df0d4f19 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi.h
+++ b/drivers/mtd/spi-nor/controllers/intel-spi.h
@@ -14,8 +14,13 @@
 struct intel_spi;
 struct resource;
 
-struct intel_spi *intel_spi_probe(struct device *dev,
-	struct resource *mem, const struct intel_spi_boardinfo *info);
+typedef struct device *(*register_platform_integrity_t)(struct device *parent,
+			enum intel_spi_type type);
+
+struct intel_spi *intel_spi_probe(struct device *dev, struct resource *mem,
+				  const struct intel_spi_boardinfo *info,
+				  register_platform_integrity_t register_pi_cb);
+
 int intel_spi_remove(struct intel_spi *ispi);
 
 #endif /* INTEL_SPI_H */
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-11-10 14:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-30 16:37 [PATCH 0/2] Platform integrity information in sysfs (version 9) Daniel Gutson
2020-09-30 16:37 ` [PATCH 1/2] " Daniel Gutson
2020-09-30 16:37 ` [PATCH 2/2] " Daniel Gutson
2020-10-02 13:43   ` Greg Kroah-Hartman
2020-10-21 19:55     ` Daniel Gutson
2020-11-10 14:07       ` Daniel Gutson
2020-10-04  4:01   ` Randy Dunlap
2020-10-22 12:08     ` Daniel Gutson
2020-09-30 16:42 ` [PATCH 0/2] " Daniel Gutson
  -- strict thread matches above, loose matches on Subject: below --
2020-09-30 13:51 [PATCH 2/2] " Daniel Gutson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).