* [PATCH v2 1/5] crypto: ccp: Represent capabilities register as a union
2024-05-28 21:07 [PATCH v2 0/5] Enable PSP security attributes on more SoCs Mario Limonciello
@ 2024-05-28 21:07 ` Mario Limonciello
2024-05-28 21:07 ` [PATCH v2 2/5] crypto: ccp: Move security attributes to their own file Mario Limonciello
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Mario Limonciello @ 2024-05-28 21:07 UTC (permalink / raw)
To: Herbert Xu
Cc: Mario Limonciello, Tom Lendacky,
open list:AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - DB...,
Richard Hughes, open list, Yazen Ghannam
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 2/5] crypto: ccp: Move security attributes to their own file
2024-05-28 21:07 [PATCH v2 0/5] Enable PSP security attributes on more SoCs Mario Limonciello
2024-05-28 21:07 ` [PATCH v2 1/5] crypto: ccp: Represent capabilities register as a union Mario Limonciello
@ 2024-05-28 21:07 ` Mario Limonciello
2024-05-29 15:20 ` Tom Lendacky
2024-05-28 21:07 ` [PATCH v2 3/5] crypto: ccp: align psp_platform_access_msg Mario Limonciello
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Mario Limonciello @ 2024-05-28 21:07 UTC (permalink / raw)
To: Herbert Xu
Cc: Mario Limonciello, Tom Lendacky,
open list:AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - DB...,
Richard Hughes, open list
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/5] crypto: ccp: Move security attributes to their own file
2024-05-28 21:07 ` [PATCH v2 2/5] crypto: ccp: Move security attributes to their own file Mario Limonciello
@ 2024-05-29 15:20 ` Tom Lendacky
2024-05-29 16:22 ` Mario Limonciello
0 siblings, 1 reply; 13+ messages in thread
From: Tom Lendacky @ 2024-05-29 15:20 UTC (permalink / raw)
To: Mario Limonciello, Herbert Xu
Cc: open list:AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - DB...,
Richard Hughes, open list
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/5] crypto: ccp: Move security attributes to their own file
2024-05-29 15:20 ` Tom Lendacky
@ 2024-05-29 16:22 ` Mario Limonciello
2024-05-29 16:50 ` Tom Lendacky
0 siblings, 1 reply; 13+ messages in thread
From: Mario Limonciello @ 2024-05-29 16:22 UTC (permalink / raw)
To: Tom Lendacky, Herbert Xu
Cc: open list:AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - DB...,
Richard Hughes, open list
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/5] crypto: ccp: Move security attributes to their own file
2024-05-29 16:22 ` Mario Limonciello
@ 2024-05-29 16:50 ` Tom Lendacky
0 siblings, 0 replies; 13+ messages in thread
From: Tom Lendacky @ 2024-05-29 16:50 UTC (permalink / raw)
To: Mario Limonciello, Herbert Xu
Cc: open list:AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - DB...,
Richard Hughes, open list
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>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/5] crypto: ccp: align psp_platform_access_msg
2024-05-28 21:07 [PATCH v2 0/5] Enable PSP security attributes on more SoCs Mario Limonciello
2024-05-28 21:07 ` [PATCH v2 1/5] crypto: ccp: Represent capabilities register as a union Mario Limonciello
2024-05-28 21:07 ` [PATCH v2 2/5] crypto: ccp: Move security attributes to their own file Mario Limonciello
@ 2024-05-28 21:07 ` Mario Limonciello
2024-05-28 21:07 ` [PATCH v2 4/5] crypto: ccp: Add support for getting security attributes on some older systems Mario Limonciello
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Mario Limonciello @ 2024-05-28 21:07 UTC (permalink / raw)
To: Herbert Xu
Cc: Mario Limonciello, Tom Lendacky,
open list:AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - DB...,
Richard Hughes, open list
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 4/5] crypto: ccp: Add support for getting security attributes on some older systems
2024-05-28 21:07 [PATCH v2 0/5] Enable PSP security attributes on more SoCs Mario Limonciello
` (2 preceding siblings ...)
2024-05-28 21:07 ` [PATCH v2 3/5] crypto: ccp: align psp_platform_access_msg Mario Limonciello
@ 2024-05-28 21:07 ` Mario Limonciello
2024-05-29 15:21 ` Tom Lendacky
2024-05-28 21:07 ` [PATCH v2 5/5] crypto: ccp: Move message about TSME being enabled later in init Mario Limonciello
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Mario Limonciello @ 2024-05-28 21:07 UTC (permalink / raw)
To: Herbert Xu
Cc: Mario Limonciello, Tom Lendacky,
open list:AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - DB...,
Richard Hughes, open list
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 5/5] crypto: ccp: Move message about TSME being enabled later in init
2024-05-28 21:07 [PATCH v2 0/5] Enable PSP security attributes on more SoCs Mario Limonciello
` (3 preceding siblings ...)
2024-05-28 21:07 ` [PATCH v2 4/5] crypto: ccp: Add support for getting security attributes on some older systems Mario Limonciello
@ 2024-05-28 21:07 ` Mario Limonciello
2024-05-29 15:22 ` Tom Lendacky
2024-05-29 8:07 ` [PATCH v2 0/5] Enable PSP security attributes on more SoCs Richard Hughes
2024-06-07 11:54 ` Herbert Xu
6 siblings, 1 reply; 13+ messages in thread
From: Mario Limonciello @ 2024-05-28 21:07 UTC (permalink / raw)
To: Herbert Xu
Cc: Mario Limonciello, Tom Lendacky,
open list:AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - DB...,
Richard Hughes, open list
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/5] Enable PSP security attributes on more SoCs
2024-05-28 21:07 [PATCH v2 0/5] Enable PSP security attributes on more SoCs Mario Limonciello
` (4 preceding siblings ...)
2024-05-28 21:07 ` [PATCH v2 5/5] crypto: ccp: Move message about TSME being enabled later in init Mario Limonciello
@ 2024-05-29 8:07 ` Richard Hughes
2024-06-07 11:54 ` Herbert Xu
6 siblings, 0 replies; 13+ messages in thread
From: Richard Hughes @ 2024-05-29 8:07 UTC (permalink / raw)
To: Mario Limonciello
Cc: Herbert Xu, Tom Lendacky,
open list:AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - DB...,
Richard Hughes, open list
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/5] Enable PSP security attributes on more SoCs
2024-05-28 21:07 [PATCH v2 0/5] Enable PSP security attributes on more SoCs Mario Limonciello
` (5 preceding siblings ...)
2024-05-29 8:07 ` [PATCH v2 0/5] Enable PSP security attributes on more SoCs Richard Hughes
@ 2024-06-07 11:54 ` Herbert Xu
6 siblings, 0 replies; 13+ messages in thread
From: Herbert Xu @ 2024-06-07 11:54 UTC (permalink / raw)
To: Mario Limonciello
Cc: Tom Lendacky,
open list:AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - DB...,
Richard Hughes, open list
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
^ permalink raw reply [flat|nested] 13+ messages in thread