Re: [PATCH] mtd: intel-spi: add is_protected and is_bios_locked knobs

2021-03-30 Thread Joe Perches
On Tue, 2021-03-30 at 18:54 +0300, Tomas Winkler wrote:
> From: Tamar Mashiah 
[]
> the region protection status is exposed via sysfs file
> as the manufacturing will need the both files in order to validate
> that the device is properly sealed.
[]
> diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c 
> b/drivers/mtd/spi-nor/controllers/intel-spi.c
[]
> +static ssize_t intel_spi_is_protected_show(struct device *dev,
> +struct device_attribute *attr, char 
> *buf)
> +{
> + struct intel_spi *ispi = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "%d", ispi->is_protected);

These should also include a newline in the format.  i.e.:

return sysfs_emit(buf, "%d\n", ispi->is_protected);


> +static ssize_t intel_spi_bios_lock_show(struct device *dev,
> + struct device_attribute *attr, char 
> *buf)
> +{
> + struct intel_spi *ispi = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "%d", ispi->is_bios_locked);

etc...




[PATCH] mtd: intel-spi: add is_protected and is_bios_locked knobs

2021-03-30 Thread Tomas Winkler
From: Tamar Mashiah 

The manufacturing access to the PCH/SOC SPI device is traditionally
performed via user space 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 device always boots with bios lock set, but during manufacturing
the bios lock has to be lifted in order to enable the write access.
So we add sysfs file (spi_bios_lock) to be able to do it on demand.
The bios lock is automatically attempted to be lifted by the driver
during initialization, this part is dropped.

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

Cc: Mika Westerberg 
Signed-off-by: Tamar Mashiah 
Signed-off-by: Tomas Winkler 
Reviewed-by: Mika Westerberg 
---
 .../ABI/testing/sysfs-devices-intel-spi   | 16 
 MAINTAINERS   |  1 +
 drivers/mfd/lpc_ich.c | 37 ++-
 .../mtd/spi-nor/controllers/intel-spi-pci.c   | 38 ++--
 drivers/mtd/spi-nor/controllers/intel-spi.c   | 96 ---
 include/linux/platform_data/x86/intel-spi.h   |  6 +-
 6 files changed, 167 insertions(+), 27 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-intel-spi

diff --git a/Documentation/ABI/testing/sysfs-devices-intel-spi 
b/Documentation/ABI/testing/sysfs-devices-intel-spi
new file mode 100644
index ..515c36f8b5cf
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-intel-spi
@@ -0,0 +1,16 @@
+What:  /sys/devices/.../is_protected
+Date:  April 2021
+Contact:   Tomas Winkler 
+Description:
+   The /sys/devices/.../is_protected attribute allows the user
+   space to check if the intel spi controller is write protected
+   from the host.
+
+What:  /sys/devices/.../bios_lock
+Date:  April 2021
+Contact:   Tomas Winkler 
+Description:
+   The /sys/devices/.../bios_lock attribute allows the user
+   space to check if the intel spi controller is locked by bios
+   for writes. It is possible to unlock the bios lock by writing
+   "unlock" to the file.
diff --git a/MAINTAINERS b/MAINTAINERS
index ba561e5bc6f0..37c934f34ed7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11571,6 +11571,7 @@ Q:  
http://patchwork.ozlabs.org/project/linux-mtd/list/
 C: irc://irc.oftc.net/mtd
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git 
mtd/fixes
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next
+F: Documentation/ABI/testing/sysfs-devices-intel-spi
 F: Documentation/devicetree/bindings/mtd/
 F: drivers/mtd/
 F: include/linux/mtd/
diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
index 3bbb29a7e7a5..eceaf11bec93 100644
--- a/drivers/mfd/lpc_ich.c
+++ b/drivers/mfd/lpc_ich.c
@@ -1083,12 +1083,39 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
return ret;
 }
 
+static int lcp_ich_bios_unlock(struct device *dev)
+{
+   struct pci_dev *pdev = to_pci_dev(dev);
+   u32 bcr = 0;
+
+   pci_read_config_dword(pdev, BCR, &bcr);
+   if (!(bcr & BCR_WPD)) {
+   bcr |= BCR_WPD;
+   pci_write_config_dword(pdev, BCR, bcr);
+   pci_read_config_dword(pdev, BCR, &bcr);
+   }
+
+   if (!(bcr & BCR_WPD))
+   return -EIO;
+
+   return 0;
+}
+
+static bool lcp_ich_is_bios_locked(struct device *dev)
+{
+   struct pci_dev *pdev = to_pci_dev(dev);
+   u32 bcr = 0;
+
+   pci_read_config_dword(pdev, BCR, &bcr);
+   return !(bcr & BCR_WPD);
+}
+
 static int lpc_ich_init_spi(struct pci_dev *dev)
 {
struct lpc_ich_priv *priv = pci_get_drvdata(dev);
struct resource *res = &intel_spi_res[0];
struct intel_spi_boardinfo *info;
-   u32 spi_base, rcba, bcr;
+   u32 spi_base, rcba;
 
info = devm_kzalloc(&dev->dev, sizeof(*info), GFP_KERNEL);
if (!info)
@@ -1112,8 +1139,8 @@ 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);
+   info->is_bios_locked = lcp_ich_is_bios_locked;
+   info->bios_unlock = lcp_ich_bios_unlock;
}
break;
 
@@ -1134,8 +1161,8 @@ static int lpc_ich_init_spi(struct pci_dev *dev)
res->start = spi_base & 0xfff0;