[v2] spi: intel: Add protected and locked attributes

Commit Message

Mika Westerberg Oct. 9, 2024, 6:22 a.m. UTC

From: Alexander Usyskin <[e-mail]>

The manufacturing access to the PCH/SoC SPI device is traditionally
performed via userspace driver accessing registers via /dev/mem but due
to security concerns /dev/mem access is being much restricted, hence the
reason for utilizing dedicated Intel PCH/SoC SPI controller driver,
which is already implemented in the Linux kernel.

Intel PCH/SoC SPI controller protects the flash storage via two
mechanisms one is the via region protection registers and second via
BIOS lock. The BIOS locks only the BIOS regions usually 0 and/or 6.

The device always boots with BIOS lock set, but during manufacturing the
BIOS lock has to be lifted in order to enable the write access. This can
be done by passing "writeable=1" in the command line when the driver is
loaded. This "locked" state is exposed through new sysfs attributes
(intel_spi_locked, intel_spi_bios_locked).

Second, also the region protection status is exposed via sysfs attribute
(intel_spi_protected) as the manufacturing will need the both files in
order to validate that the device is properly sealed.

Includes code written by Tamar Mashiah.

Signed-off-by: Alexander Usyskin <[e-mail]>
Co-developed-by: Tomas Winkler <[e-mail]>
Signed-off-by: Tomas Winkler <[e-mail]>
Reviewed-by: Andy Shevchenko <[e-mail]>
Signed-off-by: Mika Westerberg <[e-mail]>
---
Changes from v1:

  * Use correct email address for Tomas.

 .../ABI/testing/sysfs-driver-spi-intel        | 20 ++++++
 drivers/spi/spi-intel-pci.c                   |  1 +
 drivers/spi/spi-intel-platform.c              |  1 +
 drivers/spi/spi-intel.c                       | 64 +++++++++++++++++--
 drivers/spi/spi-intel.h                       |  2 +
 5 files changed, 83 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-spi-intel

Comments

Mark Brown Oct. 10, 2024, 11:14 a.m. UTC | #1

On Wed, 09 Oct 2024 09:22:44 +0300, Mika Westerberg wrote:
> The manufacturing access to the PCH/SoC SPI device is traditionally
> performed via userspace driver accessing registers via /dev/mem but due
> to security concerns /dev/mem access is being much restricted, hence the
> reason for utilizing dedicated Intel PCH/SoC SPI controller driver,
> which is already implemented in the Linux kernel.
> 
> Intel PCH/SoC SPI controller protects the flash storage via two
> mechanisms one is the via region protection registers and second via
> BIOS lock. The BIOS locks only the BIOS regions usually 0 and/or 6.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: intel: Add protected and locked attributes
      commit: b1258105f9ce5203f48a47fd2f2cec8c38c41841

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Patch

diff --git a/Documentation/ABI/testing/sysfs-driver-spi-intel b/Documentation/ABI/testing/sysfs-driver-spi-intel
new file mode 100644
index 000000000000..d7c9139ddbf3
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-spi-intel
@@ -0,0 +1,20 @@ 
+What:		/sys/devices/.../intel_spi_protected
+Date:		Feb 2025
+KernelVersion:	6.13
+Contact:	Alexander Usyskin <[e-mail]>
+Description:	This attribute allows the userspace to check if the
+		Intel SPI flash controller is write protected from the host.
+
+What:		/sys/devices/.../intel_spi_locked
+Date:		Feb 2025
+KernelVersion:	6.13
+Contact:	Alexander Usyskin <[e-mail]>
+Description:	This attribute allows the user space to check if the
+		Intel SPI flash controller locks supported opcodes.
+
+What:		/sys/devices/.../intel_spi_bios_locked
+Date:		Feb 2025
+KernelVersion:	6.13
+Contact:	Alexander Usyskin <[e-mail]>
+Description:	This attribute allows the user space to check if the
+		Intel SPI flash controller BIOS region is locked for writes.
diff --git a/drivers/spi/spi-intel-pci.c b/drivers/spi/spi-intel-pci.c
index 4337ca51d7aa..c3b54928143d 100644
--- a/drivers/spi/spi-intel-pci.c
+++ b/drivers/spi/spi-intel-pci.c
@@ -94,6 +94,7 @@  static struct pci_driver intel_spi_pci_driver = {
 	.name = "intel-spi",
 	.id_table = intel_spi_pci_ids,
 	.probe = intel_spi_pci_probe,
+	.dev_groups = intel_spi_groups,
 };
 
 module_pci_driver(intel_spi_pci_driver);
diff --git a/drivers/spi/spi-intel-platform.c b/drivers/spi/spi-intel-platform.c
index 2ef09fa35661..0974cca83a5d 100644
--- a/drivers/spi/spi-intel-platform.c
+++ b/drivers/spi/spi-intel-platform.c
@@ -28,6 +28,7 @@  static struct platform_driver intel_spi_platform_driver = {
 	.probe = intel_spi_platform_probe,
 	.driver = {
 		.name = "intel-spi",
+		.dev_groups = intel_spi_groups,
 	},
 };
 
diff --git a/drivers/spi/spi-intel.c b/drivers/spi/spi-intel.c
index 795b7e72baea..b0dcdb6fb8fa 100644
--- a/drivers/spi/spi-intel.c
+++ b/drivers/spi/spi-intel.c
@@ -148,6 +148,8 @@ 
  * @pr_num: Maximum number of protected range registers
  * @chip0_size: Size of the first flash chip in bytes
  * @locked: Is SPI setting locked
+ * @protected: Whether the regions are write protected
+ * @bios_locked: Is BIOS region locked
  * @swseq_reg: Use SW sequencer in register reads/writes
  * @swseq_erase: Use SW sequencer in erase operation
  * @atomic_preopcode: Holds preopcode when atomic sequence is requested
@@ -166,6 +168,8 @@  struct intel_spi {
 	size_t pr_num;
 	size_t chip0_size;
 	bool locked;
+	bool protected;
+	bool bios_locked;
 	bool swseq_reg;
 	bool swseq_erase;
 	u8 atomic_preopcode;
@@ -1109,10 +1113,13 @@  static int intel_spi_init(struct intel_spi *ispi)
 		return -EINVAL;
 	}
 
-	/* Try to disable write protection if user asked to do so */
-	if (writeable && !intel_spi_set_writeable(ispi)) {
-		dev_warn(ispi->dev, "can't disable chip write protection\n");
-		writeable = false;
+	ispi->bios_locked = true;
+	/* Try to disable BIOS write protection if user asked to do so */
+	if (writeable) {
+		if (intel_spi_set_writeable(ispi))
+			ispi->bios_locked = false;
+		else
+			dev_warn(ispi->dev, "can't disable chip write protection\n");
 	}
 
 	/* Disable #SMI generation from HW sequencer */
@@ -1247,8 +1254,10 @@  static void intel_spi_fill_partition(struct intel_spi *ispi,
 		 * Also if the user did not ask the chip to be writeable
 		 * mask the bit too.
 		 */
-		if (!writeable || intel_spi_is_protected(ispi, base, limit))
+		if (!writeable || intel_spi_is_protected(ispi, base, limit)) {
 			part->mask_flags |= MTD_WRITEABLE;
+			ispi->protected = true;
+		}
 
 		end = (limit << 12) + 4096;
 		if (end > part->size)
@@ -1411,6 +1420,50 @@  static int intel_spi_populate_chip(struct intel_spi *ispi)
 	return 0;
 }
 
+static ssize_t intel_spi_protected_show(struct device *dev,
+					struct device_attribute *attr, char *buf)
+{
+	struct intel_spi *ispi = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", ispi->protected);
+}
+static DEVICE_ATTR_ADMIN_RO(intel_spi_protected);
+
+static ssize_t intel_spi_locked_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct intel_spi *ispi = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", ispi->locked);
+}
+static DEVICE_ATTR_ADMIN_RO(intel_spi_locked);
+
+static ssize_t intel_spi_bios_locked_show(struct device *dev,
+					  struct device_attribute *attr, char *buf)
+{
+	struct intel_spi *ispi = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", ispi->bios_locked);
+}
+static DEVICE_ATTR_ADMIN_RO(intel_spi_bios_locked);
+
+static struct attribute *intel_spi_attrs[] = {
+	&dev_attr_intel_spi_protected.attr,
+	&dev_attr_intel_spi_locked.attr,
+	&dev_attr_intel_spi_bios_locked.attr,
+	NULL
+};
+
+static const struct attribute_group intel_spi_attr_group = {
+	.attrs = intel_spi_attrs,
+};
+
+const struct attribute_group *intel_spi_groups[] = {
+	&intel_spi_attr_group,
+	NULL
+};
+EXPORT_SYMBOL_GPL(intel_spi_groups);
+
 /**
  * intel_spi_probe() - Probe the Intel SPI flash controller
  * @dev: Pointer to the parent device
@@ -1451,6 +1504,7 @@  int intel_spi_probe(struct device *dev, struct resource *mem,
 	if (ret)
 		return ret;
 
+	dev_set_drvdata(dev, ispi);
 	return intel_spi_populate_chip(ispi);
 }
 EXPORT_SYMBOL_GPL(intel_spi_probe);
diff --git a/drivers/spi/spi-intel.h b/drivers/spi/spi-intel.h
index a4f0327a46ff..c5f35060dd63 100644
--- a/drivers/spi/spi-intel.h
+++ b/drivers/spi/spi-intel.h
@@ -13,6 +13,8 @@ 
 
 struct resource;
 
+extern const struct attribute_group *intel_spi_groups[];
+
 int intel_spi_probe(struct device *dev, struct resource *mem,
 		    const struct intel_spi_boardinfo *info);