[PATCH v2 0/5] Enable PSP security attributes on more SoCs

[PATCH v2 0/5] Enable PSP security attributes on more SoCs

[Mario Limonciello]

On some older SoCs the PSP doesn't export security attributes in the
capabilities register.  On these SoCs it is however possible to get
the information by a platform access command.

Restructure the driver to move all security attribute handling to
a central location and then add support for calling the platform
access command on those processors.

v1->v2:
 * Add tags (except patch 2)
 * Fix kernel robot reported issue
 * Move a check from patch 4 to patch 5
Mario Limonciello (5):
  crypto: ccp: Represent capabilities register as a union
  crypto: ccp: Move security attributes to their own file
  crypto: ccp: align psp_platform_access_msg
  crypto: ccp: Add support for getting security attributes on some older
    systems
  crypto: ccp: Move message about TSME being enabled later in init

 MAINTAINERS                         |   6 ++
 drivers/crypto/ccp/Makefile         |   3 +-
 drivers/crypto/ccp/dbc.c            |   2 +-
 drivers/crypto/ccp/hsti.c           | 138 ++++++++++++++++++++++++++++
 drivers/crypto/ccp/hsti.h           |  17 ++++
 drivers/crypto/ccp/psp-dev.c        |  23 ++---
 drivers/crypto/ccp/psp-dev.h        |  46 +++++-----
 drivers/crypto/ccp/sp-dev.h         |   2 +-
 drivers/crypto/ccp/sp-pci.c         |  67 ++------------
 include/linux/psp-platform-access.h |   5 +-
 10 files changed, 210 insertions(+), 99 deletions(-)
 create mode 100644 drivers/crypto/ccp/hsti.c
 create mode 100644 drivers/crypto/ccp/hsti.h

-- 
2.43.0

[PATCH v2 1/5] crypto: ccp: Represent capabilities register as a union

[Mario Limonciello]

Making the capabilities register a union makes it easier to refer
to the members instead of always doing bit shifts.

No intended functional changes.

Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
Suggested-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
 * Add tag
---
 drivers/crypto/ccp/dbc.c     |  2 +-
 drivers/crypto/ccp/psp-dev.c | 11 ++++-----
 drivers/crypto/ccp/psp-dev.h | 44 ++++++++++++++++++++----------------
 drivers/crypto/ccp/sp-dev.h  |  1 -
 drivers/crypto/ccp/sp-pci.c  | 26 ++++++++++-----------
 5 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/drivers/crypto/ccp/dbc.c b/drivers/crypto/ccp/dbc.c
index d373caab52f8..5b105a23f699 100644
--- a/drivers/crypto/ccp/dbc.c
+++ b/drivers/crypto/ccp/dbc.c
@@ -223,7 +223,7 @@ int dbc_dev_init(struct psp_device *psp)
 	dbc_dev->dev = dev;
 	dbc_dev->psp = psp;
 
-	if (PSP_CAPABILITY(psp, DBC_THRU_EXT)) {
+	if (psp->capability.dbc_thru_ext) {
 		dbc_dev->use_ext = true;
 		dbc_dev->payload_size = &dbc_dev->mbox->ext_req.header.payload_size;
 		dbc_dev->result = &dbc_dev->mbox->ext_req.header.status;
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index 56bf832c2947..7d9d2042be35 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -154,11 +154,10 @@ static unsigned int psp_get_capability(struct psp_device *psp)
 		dev_notice(psp->dev, "psp: unable to access the device: you might be running a broken BIOS.\n");
 		return -ENODEV;
 	}
-	psp->capability = val;
+	psp->capability.raw = val;
 
 	/* Detect TSME and/or SME status */
-	if (PSP_CAPABILITY(psp, PSP_SECURITY_REPORTING) &&
-	    psp->capability & (PSP_SECURITY_TSME_STATUS << PSP_CAPABILITY_PSP_SECURITY_OFFSET)) {
+	if (psp->capability.security_reporting && psp->capability.tsme_status) {
 		if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
 			dev_notice(psp->dev, "psp: Both TSME and SME are active, SME is unnecessary when TSME is active.\n");
 		else
@@ -171,7 +170,7 @@ static unsigned int psp_get_capability(struct psp_device *psp)
 static int psp_check_sev_support(struct psp_device *psp)
 {
 	/* Check if device supports SEV feature */
-	if (!PSP_CAPABILITY(psp, SEV)) {
+	if (!psp->capability.sev) {
 		dev_dbg(psp->dev, "psp does not support SEV\n");
 		return -ENODEV;
 	}
@@ -182,7 +181,7 @@ static int psp_check_sev_support(struct psp_device *psp)
 static int psp_check_tee_support(struct psp_device *psp)
 {
 	/* Check if device supports TEE feature */
-	if (!PSP_CAPABILITY(psp, TEE)) {
+	if (!psp->capability.tee) {
 		dev_dbg(psp->dev, "psp does not support TEE\n");
 		return -ENODEV;
 	}
@@ -214,7 +213,7 @@ static int psp_init(struct psp_device *psp)
 
 	/* dbc must come after platform access as it tests the feature */
 	if (PSP_FEATURE(psp, DBC) ||
-	    PSP_CAPABILITY(psp, DBC_THRU_EXT)) {
+	    psp->capability.dbc_thru_ext) {
 		ret = dbc_dev_init(psp);
 		if (ret)
 			return ret;
diff --git a/drivers/crypto/ccp/psp-dev.h b/drivers/crypto/ccp/psp-dev.h
index ae582ba63729..02a7c94c02df 100644
--- a/drivers/crypto/ccp/psp-dev.h
+++ b/drivers/crypto/ccp/psp-dev.h
@@ -26,6 +26,29 @@ extern struct psp_device *psp_master;
 
 typedef void (*psp_irq_handler_t)(int, void *, unsigned int);
 
+union psp_cap_register {
+	unsigned int raw;
+	struct {
+		unsigned int sev			:1,
+			     tee			:1,
+			     dbc_thru_ext		:1,
+			     rsvd1			:4,
+			     security_reporting		:1,
+			     fused_part			:1,
+			     rsvd2			:1,
+			     debug_lock_on		:1,
+			     rsvd3			:2,
+			     tsme_status		:1,
+			     rsvd4			:1,
+			     anti_rollback_status	:1,
+			     rpmc_production_enabled	:1,
+			     rpmc_spirom_available	:1,
+			     hsp_tpm_available		:1,
+			     rom_armor_enforced		:1,
+			     rsvd5			:12;
+	};
+};
+
 struct psp_device {
 	struct list_head entry;
 
@@ -46,7 +69,7 @@ struct psp_device {
 	void *platform_access_data;
 	void *dbc_data;
 
-	unsigned int capability;
+	union psp_cap_register capability;
 };
 
 void psp_set_sev_irq_handler(struct psp_device *psp, psp_irq_handler_t handler,
@@ -55,26 +78,7 @@ void psp_clear_sev_irq_handler(struct psp_device *psp);
 
 struct psp_device *psp_get_master_device(void);
 
-#define PSP_CAPABILITY_SEV			BIT(0)
-#define PSP_CAPABILITY_TEE			BIT(1)
-#define PSP_CAPABILITY_DBC_THRU_EXT		BIT(2)
-#define PSP_CAPABILITY_PSP_SECURITY_REPORTING	BIT(7)
-
 #define PSP_CAPABILITY_PSP_SECURITY_OFFSET	8
-/*
- * The PSP doesn't directly store these bits in the capability register
- * but instead copies them from the results of query command.
- *
- * The offsets from the query command are below, and shifted when used.
- */
-#define PSP_SECURITY_FUSED_PART			BIT(0)
-#define PSP_SECURITY_DEBUG_LOCK_ON		BIT(2)
-#define PSP_SECURITY_TSME_STATUS		BIT(5)
-#define PSP_SECURITY_ANTI_ROLLBACK_STATUS	BIT(7)
-#define PSP_SECURITY_RPMC_PRODUCTION_ENABLED	BIT(8)
-#define PSP_SECURITY_RPMC_SPIROM_AVAILABLE	BIT(9)
-#define PSP_SECURITY_HSP_TPM_AVAILABLE		BIT(10)
-#define PSP_SECURITY_ROM_ARMOR_ENFORCED		BIT(11)
 
 /**
  * enum psp_cmd - PSP mailbox commands
diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h
index 03d5b9e04084..c4e125efe6c7 100644
--- a/drivers/crypto/ccp/sp-dev.h
+++ b/drivers/crypto/ccp/sp-dev.h
@@ -30,7 +30,6 @@
 
 #define PLATFORM_FEATURE_DBC		0x1
 
-#define PSP_CAPABILITY(psp, cap) (psp->capability & PSP_CAPABILITY_##cap)
 #define PSP_FEATURE(psp, feat)	(psp->vdata && psp->vdata->platform_features & PLATFORM_FEATURE_##feat)
 
 /* Structure to hold CCP device data */
diff --git a/drivers/crypto/ccp/sp-pci.c b/drivers/crypto/ccp/sp-pci.c
index 300dda14182b..b57392292af1 100644
--- a/drivers/crypto/ccp/sp-pci.c
+++ b/drivers/crypto/ccp/sp-pci.c
@@ -39,31 +39,30 @@ struct sp_pci {
 };
 static struct sp_device *sp_dev_master;
 
-#define security_attribute_show(name, def)					\
+#define security_attribute_show(name)						\
 static ssize_t name##_show(struct device *d, struct device_attribute *attr,	\
 			   char *buf)						\
 {										\
 	struct sp_device *sp = dev_get_drvdata(d);				\
 	struct psp_device *psp = sp->psp_data;					\
-	int bit = PSP_SECURITY_##def << PSP_CAPABILITY_PSP_SECURITY_OFFSET;	\
-	return sysfs_emit(buf, "%d\n", (psp->capability & bit) > 0);		\
+	return sysfs_emit(buf, "%d\n", psp->capability.name);			\
 }
 
-security_attribute_show(fused_part, FUSED_PART)
+security_attribute_show(fused_part)
 static DEVICE_ATTR_RO(fused_part);
-security_attribute_show(debug_lock_on, DEBUG_LOCK_ON)
+security_attribute_show(debug_lock_on)
 static DEVICE_ATTR_RO(debug_lock_on);
-security_attribute_show(tsme_status, TSME_STATUS)
+security_attribute_show(tsme_status)
 static DEVICE_ATTR_RO(tsme_status);
-security_attribute_show(anti_rollback_status, ANTI_ROLLBACK_STATUS)
+security_attribute_show(anti_rollback_status)
 static DEVICE_ATTR_RO(anti_rollback_status);
-security_attribute_show(rpmc_production_enabled, RPMC_PRODUCTION_ENABLED)
+security_attribute_show(rpmc_production_enabled)
 static DEVICE_ATTR_RO(rpmc_production_enabled);
-security_attribute_show(rpmc_spirom_available, RPMC_SPIROM_AVAILABLE)
+security_attribute_show(rpmc_spirom_available)
 static DEVICE_ATTR_RO(rpmc_spirom_available);
-security_attribute_show(hsp_tpm_available, HSP_TPM_AVAILABLE)
+security_attribute_show(hsp_tpm_available)
 static DEVICE_ATTR_RO(hsp_tpm_available);
-security_attribute_show(rom_armor_enforced, ROM_ARMOR_ENFORCED)
+security_attribute_show(rom_armor_enforced)
 static DEVICE_ATTR_RO(rom_armor_enforced);
 
 static struct attribute *psp_security_attrs[] = {
@@ -84,7 +83,7 @@ static umode_t psp_security_is_visible(struct kobject *kobj, struct attribute *a
 	struct sp_device *sp = dev_get_drvdata(dev);
 	struct psp_device *psp = sp->psp_data;
 
-	if (psp && PSP_CAPABILITY(psp, PSP_SECURITY_REPORTING))
+	if (psp && psp->capability.security_reporting)
 		return 0444;
 
 	return 0;
@@ -134,8 +133,7 @@ static umode_t psp_firmware_is_visible(struct kobject *kobj, struct attribute *a
 	    psp->vdata->bootloader_info_reg)
 		val = ioread32(psp->io_regs + psp->vdata->bootloader_info_reg);
 
-	if (attr == &dev_attr_tee_version.attr &&
-	    PSP_CAPABILITY(psp, TEE) &&
+	if (attr == &dev_attr_tee_version.attr && psp->capability.tee &&
 	    psp->vdata->tee->info_reg)
 		val = ioread32(psp->io_regs + psp->vdata->tee->info_reg);
 
-- 
2.43.0

[PATCH v2 2/5] crypto: ccp: Move security attributes to their own file

[Mario Limonciello]

To prepare for other code that will manipulate security attributes
move the handling code out of sp-pci.c. No intended functional changes.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
 * Only add psp_security_attr_group when PSP support enabled
   (Fixes lkp robot reported Kconfig issue)
---
 MAINTAINERS                  |  6 ++++
 drivers/crypto/ccp/Makefile  |  3 +-
 drivers/crypto/ccp/hsti.c    | 68 ++++++++++++++++++++++++++++++++++++
 drivers/crypto/ccp/hsti.h    | 15 ++++++++
 drivers/crypto/ccp/psp-dev.c |  1 +
 drivers/crypto/ccp/sp-pci.c  | 58 ++----------------------------
 6 files changed, 95 insertions(+), 56 deletions(-)
 create mode 100644 drivers/crypto/ccp/hsti.c
 create mode 100644 drivers/crypto/ccp/hsti.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d6c90161c7bf..883fb3b246b6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -991,6 +991,12 @@ F:	include/uapi/linux/psp-dbc.h
 F:	tools/crypto/ccp/*.c
 F:	tools/crypto/ccp/*.py
 
+AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - HSTI SUPPORT
+M:	Mario Limonciello <mario.limonciello@amd.com>
+L:	linux-crypto@vger.kernel.org
+S:	Supported
+F:	drivers/crypto/ccp/hsti.*
+
 AMD DISPLAY CORE
 M:	Harry Wentland <harry.wentland@amd.com>
 M:	Leo Li <sunpeng.li@amd.com>
diff --git a/drivers/crypto/ccp/Makefile b/drivers/crypto/ccp/Makefile
index aa0ba2d17e1e..394484929dae 100644
--- a/drivers/crypto/ccp/Makefile
+++ b/drivers/crypto/ccp/Makefile
@@ -12,7 +12,8 @@ ccp-$(CONFIG_CRYPTO_DEV_SP_PSP) += psp-dev.o \
                                    sev-dev.o \
                                    tee-dev.o \
                                    platform-access.o \
-                                   dbc.o
+                                   dbc.o \
+                                   hsti.o
 
 obj-$(CONFIG_CRYPTO_DEV_CCP_CRYPTO) += ccp-crypto.o
 ccp-crypto-objs := ccp-crypto-main.o \
diff --git a/drivers/crypto/ccp/hsti.c b/drivers/crypto/ccp/hsti.c
new file mode 100644
index 000000000000..076c1d175b2b
--- /dev/null
+++ b/drivers/crypto/ccp/hsti.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AMD Secure Processor device driver, security attributes
+ *
+ * Copyright (C) 2023-2024 Advanced Micro Devices, Inc.
+ *
+ * Author: Mario Limonciello <mario.limonciello@amd.com>
+ */
+
+#include <linux/device.h>
+
+#include "psp-dev.h"
+#include "hsti.h"
+
+#define security_attribute_show(name)						\
+static ssize_t name##_show(struct device *d, struct device_attribute *attr,	\
+			   char *buf)						\
+{										\
+	struct sp_device *sp = dev_get_drvdata(d);				\
+	struct psp_device *psp = sp->psp_data;					\
+	return sysfs_emit(buf, "%d\n", psp->capability.name);		\
+}
+
+security_attribute_show(fused_part)
+static DEVICE_ATTR_RO(fused_part);
+security_attribute_show(debug_lock_on)
+static DEVICE_ATTR_RO(debug_lock_on);
+security_attribute_show(tsme_status)
+static DEVICE_ATTR_RO(tsme_status);
+security_attribute_show(anti_rollback_status)
+static DEVICE_ATTR_RO(anti_rollback_status);
+security_attribute_show(rpmc_production_enabled)
+static DEVICE_ATTR_RO(rpmc_production_enabled);
+security_attribute_show(rpmc_spirom_available)
+static DEVICE_ATTR_RO(rpmc_spirom_available);
+security_attribute_show(hsp_tpm_available)
+static DEVICE_ATTR_RO(hsp_tpm_available);
+security_attribute_show(rom_armor_enforced)
+static DEVICE_ATTR_RO(rom_armor_enforced);
+
+static struct attribute *psp_security_attrs[] = {
+	&dev_attr_fused_part.attr,
+	&dev_attr_debug_lock_on.attr,
+	&dev_attr_tsme_status.attr,
+	&dev_attr_anti_rollback_status.attr,
+	&dev_attr_rpmc_production_enabled.attr,
+	&dev_attr_rpmc_spirom_available.attr,
+	&dev_attr_hsp_tpm_available.attr,
+	&dev_attr_rom_armor_enforced.attr,
+	NULL
+};
+
+static umode_t psp_security_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct sp_device *sp = dev_get_drvdata(dev);
+	struct psp_device *psp = sp->psp_data;
+
+	if (psp && psp->capability.security_reporting)
+		return 0444;
+
+	return 0;
+}
+
+struct attribute_group psp_security_attr_group = {
+	.attrs = psp_security_attrs,
+	.is_visible = psp_security_is_visible,
+};
diff --git a/drivers/crypto/ccp/hsti.h b/drivers/crypto/ccp/hsti.h
new file mode 100644
index 000000000000..e5c5ceab9973
--- /dev/null
+++ b/drivers/crypto/ccp/hsti.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * AMD Secure Processor device driver, security attributes
+ *
+ * Copyright (C) 2023-2024 Advanced Micro Devices, Inc.
+ *
+ * Author: Mario Limonciello <mario.limonciello@amd.com>
+ */
+
+#ifndef __HSTI_H
+#define __HSTI_H
+
+extern struct attribute_group psp_security_attr_group;
+
+#endif /* __HSTI_H */
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index 7d9d2042be35..1a7b991c27f7 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -19,6 +19,7 @@
 #include "tee-dev.h"
 #include "platform-access.h"
 #include "dbc.h"
+#include "hsti.h"
 
 struct psp_device *psp_master;
 
diff --git a/drivers/crypto/ccp/sp-pci.c b/drivers/crypto/ccp/sp-pci.c
index b57392292af1..dd31e791156d 100644
--- a/drivers/crypto/ccp/sp-pci.c
+++ b/drivers/crypto/ccp/sp-pci.c
@@ -24,6 +24,7 @@
 
 #include "ccp-dev.h"
 #include "psp-dev.h"
+#include "hsti.h"
 
 /* used for version string AA.BB.CC.DD */
 #define AA				GENMASK(31, 24)
@@ -39,61 +40,6 @@ struct sp_pci {
 };
 static struct sp_device *sp_dev_master;
 
-#define security_attribute_show(name)						\
-static ssize_t name##_show(struct device *d, struct device_attribute *attr,	\
-			   char *buf)						\
-{										\
-	struct sp_device *sp = dev_get_drvdata(d);				\
-	struct psp_device *psp = sp->psp_data;					\
-	return sysfs_emit(buf, "%d\n", psp->capability.name);			\
-}
-
-security_attribute_show(fused_part)
-static DEVICE_ATTR_RO(fused_part);
-security_attribute_show(debug_lock_on)
-static DEVICE_ATTR_RO(debug_lock_on);
-security_attribute_show(tsme_status)
-static DEVICE_ATTR_RO(tsme_status);
-security_attribute_show(anti_rollback_status)
-static DEVICE_ATTR_RO(anti_rollback_status);
-security_attribute_show(rpmc_production_enabled)
-static DEVICE_ATTR_RO(rpmc_production_enabled);
-security_attribute_show(rpmc_spirom_available)
-static DEVICE_ATTR_RO(rpmc_spirom_available);
-security_attribute_show(hsp_tpm_available)
-static DEVICE_ATTR_RO(hsp_tpm_available);
-security_attribute_show(rom_armor_enforced)
-static DEVICE_ATTR_RO(rom_armor_enforced);
-
-static struct attribute *psp_security_attrs[] = {
-	&dev_attr_fused_part.attr,
-	&dev_attr_debug_lock_on.attr,
-	&dev_attr_tsme_status.attr,
-	&dev_attr_anti_rollback_status.attr,
-	&dev_attr_rpmc_production_enabled.attr,
-	&dev_attr_rpmc_spirom_available.attr,
-	&dev_attr_hsp_tpm_available.attr,
-	&dev_attr_rom_armor_enforced.attr,
-	NULL
-};
-
-static umode_t psp_security_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
-{
-	struct device *dev = kobj_to_dev(kobj);
-	struct sp_device *sp = dev_get_drvdata(dev);
-	struct psp_device *psp = sp->psp_data;
-
-	if (psp && psp->capability.security_reporting)
-		return 0444;
-
-	return 0;
-}
-
-static struct attribute_group psp_security_attr_group = {
-	.attrs = psp_security_attrs,
-	.is_visible = psp_security_is_visible,
-};
-
 #define version_attribute_show(name, _offset)					\
 static ssize_t name##_show(struct device *d, struct device_attribute *attr,	\
 			   char *buf)						\
@@ -150,7 +96,9 @@ static struct attribute_group psp_firmware_attr_group = {
 };
 
 static const struct attribute_group *psp_groups[] = {
+#ifdef CONFIG_CRYPTO_DEV_SP_PSP
 	&psp_security_attr_group,
+#endif
 	&psp_firmware_attr_group,
 	NULL,
 };
-- 
2.43.0

[PATCH v2 3/5] crypto: ccp: align psp_platform_access_msg

[Mario Limonciello]

Align the whitespace so that future messages will also be better
aligned.

Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
 * Add tag
---
 include/linux/psp-platform-access.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/psp-platform-access.h b/include/linux/psp-platform-access.h
index c1dc87fc536b..23893b33e48c 100644
--- a/include/linux/psp-platform-access.h
+++ b/include/linux/psp-platform-access.h
@@ -6,8 +6,8 @@
 #include <linux/psp.h>
 
 enum psp_platform_access_msg {
-	PSP_CMD_NONE = 0x0,
-	PSP_I2C_REQ_BUS_CMD = 0x64,
+	PSP_CMD_NONE			= 0x0,
+	PSP_I2C_REQ_BUS_CMD		= 0x64,
 	PSP_DYNAMIC_BOOST_GET_NONCE,
 	PSP_DYNAMIC_BOOST_SET_UID,
 	PSP_DYNAMIC_BOOST_GET_PARAMETER,
-- 
2.43.0

[PATCH v2 4/5] crypto: ccp: Add support for getting security attributes on some older systems

[Mario Limonciello]

Older systems will not populate the security attributes in the
capabilities register. The PSP on these systems, however, does have a
command to get the security attributes. Use this command during ccp
startup to populate the attributes if they're missing.

Closes: https://github.com/fwupd/fwupd/issues/5284
Closes: https://github.com/fwupd/fwupd/issues/5675
Closes: https://github.com/fwupd/fwupd/issues/6253
Closes: https://github.com/fwupd/fwupd/issues/7280
Closes: https://github.com/fwupd/fwupd/issues/6323
Closes: https://github.com/fwupd/fwupd/discussions/5433
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
 * Move code to patch 5
---
 drivers/crypto/ccp/hsti.c           | 55 +++++++++++++++++++++++++++++
 drivers/crypto/ccp/hsti.h           |  2 ++
 drivers/crypto/ccp/psp-dev.c        |  5 +++
 drivers/crypto/ccp/psp-dev.h        |  2 --
 drivers/crypto/ccp/sp-dev.h         |  1 +
 drivers/crypto/ccp/sp-pci.c         |  5 ++-
 include/linux/psp-platform-access.h |  1 +
 7 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/ccp/hsti.c b/drivers/crypto/ccp/hsti.c
index 076c1d175b2b..8b99bbd4efe2 100644
--- a/drivers/crypto/ccp/hsti.c
+++ b/drivers/crypto/ccp/hsti.c
@@ -12,6 +12,13 @@
 #include "psp-dev.h"
 #include "hsti.h"
 
+#define PSP_CAPABILITY_PSP_SECURITY_OFFSET	8
+
+struct hsti_request {
+	struct psp_req_buffer_hdr header;
+	u32 hsti;
+} __packed;
+
 #define security_attribute_show(name)						\
 static ssize_t name##_show(struct device *d, struct device_attribute *attr,	\
 			   char *buf)						\
@@ -66,3 +73,51 @@ struct attribute_group psp_security_attr_group = {
 	.attrs = psp_security_attrs,
 	.is_visible = psp_security_is_visible,
 };
+
+static int psp_poulate_hsti(struct psp_device *psp)
+{
+	struct hsti_request *req;
+	int ret;
+
+	/* Are the security attributes already reported? */
+	if (psp->capability.security_reporting)
+		return 0;
+
+	/* Allocate command-response buffer */
+	req = kzalloc(sizeof(*req), GFP_KERNEL | __GFP_ZERO);
+	if (!req)
+		return -ENOMEM;
+
+	req->header.payload_size = sizeof(req);
+
+	ret = psp_send_platform_access_msg(PSP_CMD_HSTI_QUERY, (struct psp_request *)req);
+	if (ret)
+		goto out;
+
+	if (req->header.status != 0) {
+		dev_dbg(psp->dev, "failed to populate HSTI state: %d\n", req->header.status);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	psp->capability.security_reporting = 1;
+	psp->capability.raw |= req->hsti << PSP_CAPABILITY_PSP_SECURITY_OFFSET;
+
+out:
+	kfree(req);
+
+	return ret;
+}
+
+int psp_init_hsti(struct psp_device *psp)
+{
+	int ret;
+
+	if (PSP_FEATURE(psp, HSTI)) {
+		ret = psp_poulate_hsti(psp);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/crypto/ccp/hsti.h b/drivers/crypto/ccp/hsti.h
index e5c5ceab9973..6a70f922d2c4 100644
--- a/drivers/crypto/ccp/hsti.h
+++ b/drivers/crypto/ccp/hsti.h
@@ -12,4 +12,6 @@
 
 extern struct attribute_group psp_security_attr_group;
 
+int psp_init_hsti(struct psp_device *psp);
+
 #endif /* __HSTI_H */
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index 1a7b991c27f7..0a01ad134609 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -220,6 +220,11 @@ static int psp_init(struct psp_device *psp)
 			return ret;
 	}
 
+	/* HSTI uses platform access on some systems. */
+	ret = psp_init_hsti(psp);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
diff --git a/drivers/crypto/ccp/psp-dev.h b/drivers/crypto/ccp/psp-dev.h
index 02a7c94c02df..e43ce87ede76 100644
--- a/drivers/crypto/ccp/psp-dev.h
+++ b/drivers/crypto/ccp/psp-dev.h
@@ -78,8 +78,6 @@ void psp_clear_sev_irq_handler(struct psp_device *psp);
 
 struct psp_device *psp_get_master_device(void);
 
-#define PSP_CAPABILITY_PSP_SECURITY_OFFSET	8
-
 /**
  * enum psp_cmd - PSP mailbox commands
  * @PSP_CMD_TEE_RING_INIT:	Initialize TEE ring buffer
diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h
index c4e125efe6c7..0895de823674 100644
--- a/drivers/crypto/ccp/sp-dev.h
+++ b/drivers/crypto/ccp/sp-dev.h
@@ -29,6 +29,7 @@
 #define CACHE_WB_NO_ALLOC		0xb7
 
 #define PLATFORM_FEATURE_DBC		0x1
+#define PLATFORM_FEATURE_HSTI		0x2
 
 #define PSP_FEATURE(psp, feat)	(psp->vdata && psp->vdata->platform_features & PLATFORM_FEATURE_##feat)
 
diff --git a/drivers/crypto/ccp/sp-pci.c b/drivers/crypto/ccp/sp-pci.c
index dd31e791156d..248d98fd8c48 100644
--- a/drivers/crypto/ccp/sp-pci.c
+++ b/drivers/crypto/ccp/sp-pci.c
@@ -397,10 +397,12 @@ static const struct psp_vdata pspv1 = {
 
 static const struct psp_vdata pspv2 = {
 	.sev			= &sevv2,
+	.platform_access	= &pa_v1,
 	.bootloader_info_reg	= 0x109ec,	/* C2PMSG_59 */
 	.feature_reg		= 0x109fc,	/* C2PMSG_63 */
 	.inten_reg		= 0x10690,	/* P2CMSG_INTEN */
 	.intsts_reg		= 0x10694,	/* P2CMSG_INTSTS */
+	.platform_features	= PLATFORM_FEATURE_HSTI,
 };
 
 static const struct psp_vdata pspv3 = {
@@ -413,7 +415,8 @@ static const struct psp_vdata pspv3 = {
 	.feature_reg		= 0x109fc,	/* C2PMSG_63 */
 	.inten_reg		= 0x10690,	/* P2CMSG_INTEN */
 	.intsts_reg		= 0x10694,	/* P2CMSG_INTSTS */
-	.platform_features	= PLATFORM_FEATURE_DBC,
+	.platform_features	= PLATFORM_FEATURE_DBC |
+				  PLATFORM_FEATURE_HSTI,
 };
 
 static const struct psp_vdata pspv4 = {
diff --git a/include/linux/psp-platform-access.h b/include/linux/psp-platform-access.h
index 23893b33e48c..1504fb012c05 100644
--- a/include/linux/psp-platform-access.h
+++ b/include/linux/psp-platform-access.h
@@ -7,6 +7,7 @@
 
 enum psp_platform_access_msg {
 	PSP_CMD_NONE			= 0x0,
+	PSP_CMD_HSTI_QUERY		= 0x14,
 	PSP_I2C_REQ_BUS_CMD		= 0x64,
 	PSP_DYNAMIC_BOOST_GET_NONCE,
 	PSP_DYNAMIC_BOOST_SET_UID,
-- 
2.43.0

[PATCH v2 5/5] crypto: ccp: Move message about TSME being enabled later in init

[Mario Limonciello]

Some of the security attributes data is now populated from an HSTI
command on some processors, so show the message after it has been
populated.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
 * Move code from patch 4
---
 drivers/crypto/ccp/hsti.c    | 15 +++++++++++++++
 drivers/crypto/ccp/psp-dev.c |  8 --------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/ccp/hsti.c b/drivers/crypto/ccp/hsti.c
index 8b99bbd4efe2..1b39a4fb55c0 100644
--- a/drivers/crypto/ccp/hsti.c
+++ b/drivers/crypto/ccp/hsti.c
@@ -119,5 +119,20 @@ int psp_init_hsti(struct psp_device *psp)
 			return ret;
 	}
 
+	/*
+	 * At this stage, if security information hasn't been populated by
+	 * either the PSP or by the driver through the platform command,
+	 * then there is nothing more to do.
+	 */
+	if (!psp->capability.security_reporting)
+		return 0;
+
+	if (psp->capability.tsme_status) {
+		if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
+			dev_notice(psp->dev, "psp: Both TSME and SME are active, SME is unnecessary when TSME is active.\n");
+		else
+			dev_notice(psp->dev, "psp: TSME enabled\n");
+	}
+
 	return 0;
 }
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index 0a01ad134609..1c5a7189631e 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -157,14 +157,6 @@ static unsigned int psp_get_capability(struct psp_device *psp)
 	}
 	psp->capability.raw = val;
 
-	/* Detect TSME and/or SME status */
-	if (psp->capability.security_reporting && psp->capability.tsme_status) {
-		if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
-			dev_notice(psp->dev, "psp: Both TSME and SME are active, SME is unnecessary when TSME is active.\n");
-		else
-			dev_notice(psp->dev, "psp: TSME enabled\n");
-	}
-
 	return 0;
 }
 
-- 
2.43.0

Re: [PATCH v2 0/5] Enable PSP security attributes on more SoCs

[Richard Hughes]

On Tuesday, May 28th, 2024 at 10:07 PM, Mario Limonciello <mario.limonciello@amd.com> wrote:
> v1->v2:

Looks great, and the output of "fwupdmgr security" on my pre-production Lenovo T14 (AMD Ryzen 7 PRO 4750U) is now a lot more accurate:

 HSI-1
 ✔ BIOS firmware updates:         Enabled
+✔ Fused platform:                Locked
+✔ Supported CPU:                 Valid
 ✔ TPM empty PCRs:                Valid
 ✔ TPM v2.0:                      Found
 ✔ UEFI bootservice variables:    Locked
 ✔ UEFI platform key:             Valid
 
 HSI-2
 ✔ IOMMU:                         Enabled
+✔ Platform debugging:            Locked
 ✔ TPM PCR0 reconstruction:       Valid
+✘ SPI write protection:          Disabled
 ✘ BIOS rollback protection:      Disabled
 
 HSI-3
+✘ SPI replay protection:         Not supported
 ✘ CET Platform:                  Not supported
 ✘ Pre-boot DMA protection:       Disabled
 ✘ Suspend-to-idle:               Disabled
 ✘ Suspend-to-ram:                Enabled
 
 HSI-4
+✔ Encrypted RAM:                 Encrypted
 ✔ SMAP:                          Enabled
+✘ Processor rollback protection: Disabled

Tested-by: Richard Hughes <richard@hughsie.com>

Richard

Re: [PATCH v2 2/5] crypto: ccp: Move security attributes to their own file

[Tom Lendacky]

On 5/28/24 16:07, Mario Limonciello wrote:
> To prepare for other code that will manipulate security attributes
> move the handling code out of sp-pci.c. No intended functional changes.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>   * Only add psp_security_attr_group when PSP support enabled
>     (Fixes lkp robot reported Kconfig issue)

Just verifying that there's no change in behavior in regards to whether 
the attributes are shown. Previously the attributes were not shown if 
CONFIG_CRYPTO_DEV_SP_PSP was "n" because the psp value would be NULL in 
psp_security_is_visible(), right?

Thanks,
Tom

> ---
>   MAINTAINERS                  |  6 ++++
>   drivers/crypto/ccp/Makefile  |  3 +-
>   drivers/crypto/ccp/hsti.c    | 68 ++++++++++++++++++++++++++++++++++++
>   drivers/crypto/ccp/hsti.h    | 15 ++++++++
>   drivers/crypto/ccp/psp-dev.c |  1 +
>   drivers/crypto/ccp/sp-pci.c  | 58 ++----------------------------
>   6 files changed, 95 insertions(+), 56 deletions(-)
>   create mode 100644 drivers/crypto/ccp/hsti.c
>   create mode 100644 drivers/crypto/ccp/hsti.h
> 

Re: [PATCH v2 4/5] crypto: ccp: Add support for getting security attributes on some older systems

[Tom Lendacky]

On 5/28/24 16:07, Mario Limonciello wrote:
> Older systems will not populate the security attributes in the
> capabilities register. The PSP on these systems, however, does have a
> command to get the security attributes. Use this command during ccp
> startup to populate the attributes if they're missing.
> 
> Closes: https://github.com/fwupd/fwupd/issues/5284
> Closes: https://github.com/fwupd/fwupd/issues/5675
> Closes: https://github.com/fwupd/fwupd/issues/6253
> Closes: https://github.com/fwupd/fwupd/issues/7280
> Closes: https://github.com/fwupd/fwupd/issues/6323
> Closes: https://github.com/fwupd/fwupd/discussions/5433
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Acked-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
> v1->v2:
>   * Move code to patch 5
> ---
>   drivers/crypto/ccp/hsti.c           | 55 +++++++++++++++++++++++++++++
>   drivers/crypto/ccp/hsti.h           |  2 ++
>   drivers/crypto/ccp/psp-dev.c        |  5 +++
>   drivers/crypto/ccp/psp-dev.h        |  2 --
>   drivers/crypto/ccp/sp-dev.h         |  1 +
>   drivers/crypto/ccp/sp-pci.c         |  5 ++-
>   include/linux/psp-platform-access.h |  1 +
>   7 files changed, 68 insertions(+), 3 deletions(-)
> 

Re: [PATCH v2 5/5] crypto: ccp: Move message about TSME being enabled later in init

[Tom Lendacky]

On 5/28/24 16:07, Mario Limonciello wrote:
> Some of the security attributes data is now populated from an HSTI
> command on some processors, so show the message after it has been
> populated.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Acked-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
> v1->v2:
>   * Move code from patch 4
> ---
>   drivers/crypto/ccp/hsti.c    | 15 +++++++++++++++
>   drivers/crypto/ccp/psp-dev.c |  8 --------
>   2 files changed, 15 insertions(+), 8 deletions(-)
> 

Re: [PATCH v2 2/5] crypto: ccp: Move security attributes to their own file

[Mario Limonciello]

On 5/29/2024 10:20, Tom Lendacky wrote:
> On 5/28/24 16:07, Mario Limonciello wrote:
>> To prepare for other code that will manipulate security attributes
>> move the handling code out of sp-pci.c. No intended functional changes.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> v1->v2:
>>   * Only add psp_security_attr_group when PSP support enabled
>>     (Fixes lkp robot reported Kconfig issue)
> 
> Just verifying that there's no change in behavior in regards to whether 
> the attributes are shown. Previously the attributes were not shown if 
> CONFIG_CRYPTO_DEV_SP_PSP was "n" because the psp value would be NULL in 
> psp_security_is_visible(), right?

Yes; that's right.

Re: [PATCH v2 2/5] crypto: ccp: Move security attributes to their own file

[Tom Lendacky]

On 5/29/24 11:22, Mario Limonciello wrote:
> On 5/29/2024 10:20, Tom Lendacky wrote:
>> On 5/28/24 16:07, Mario Limonciello wrote:
>>> To prepare for other code that will manipulate security attributes
>>> move the handling code out of sp-pci.c. No intended functional changes.
>>>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>>> v1->v2:
>>>   * Only add psp_security_attr_group when PSP support enabled
>>>     (Fixes lkp robot reported Kconfig issue)
>>
>> Just verifying that there's no change in behavior in regards to 
>> whether the attributes are shown. Previously the attributes were not 
>> shown if CONFIG_CRYPTO_DEV_SP_PSP was "n" because the psp value would 
>> be NULL in psp_security_is_visible(), right?
> 
> Yes; that's right.

Acked-by: Tom Lendacky <thomas.lendacky@amd.com>

> 

Re: [PATCH v2 0/5] Enable PSP security attributes on more SoCs

[Herbert Xu]

On Tue, May 28, 2024 at 04:07:07PM -0500, Mario Limonciello wrote:
> On some older SoCs the PSP doesn't export security attributes in the
> capabilities register.  On these SoCs it is however possible to get
> the information by a platform access command.
> 
> Restructure the driver to move all security attribute handling to
> a central location and then add support for calling the platform
> access command on those processors.
> 
> v1->v2:
>  * Add tags (except patch 2)
>  * Fix kernel robot reported issue
>  * Move a check from patch 4 to patch 5
> Mario Limonciello (5):
>   crypto: ccp: Represent capabilities register as a union
>   crypto: ccp: Move security attributes to their own file
>   crypto: ccp: align psp_platform_access_msg
>   crypto: ccp: Add support for getting security attributes on some older
>     systems
>   crypto: ccp: Move message about TSME being enabled later in init
> 
>  MAINTAINERS                         |   6 ++
>  drivers/crypto/ccp/Makefile         |   3 +-
>  drivers/crypto/ccp/dbc.c            |   2 +-
>  drivers/crypto/ccp/hsti.c           | 138 ++++++++++++++++++++++++++++
>  drivers/crypto/ccp/hsti.h           |  17 ++++
>  drivers/crypto/ccp/psp-dev.c        |  23 ++---
>  drivers/crypto/ccp/psp-dev.h        |  46 +++++-----
>  drivers/crypto/ccp/sp-dev.h         |   2 +-
>  drivers/crypto/ccp/sp-pci.c         |  67 ++------------
>  include/linux/psp-platform-access.h |   5 +-
>  10 files changed, 210 insertions(+), 99 deletions(-)
>  create mode 100644 drivers/crypto/ccp/hsti.c
>  create mode 100644 drivers/crypto/ccp/hsti.h
> 
> -- 
> 2.43.0

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt