Skip to contentSkip to conversation

Support ACPI PSP on Hyper-V

PATCH Superseded Jeremi Piotrowski 3 participants 12 messages Feb 13, 2023 – Feb 20, 2023

Subsystems (5)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover Support ACPI PSP on Hyper-V

This patch series introduces support for discovering AMD's PSP from an ACPI table and extends the CCP driver to allow binding to that device on x86. This method of PSP discovery is used on Hyper-V when SNP isolation support is exposed to the guest. There is no ACPI node associated with this PSP, so after parsing the ASPT it is registered with the system as a platform_device.

I thought about putting psp.c in arch/x86/coco, but that directory is meant for the (confidential) guest side of CoCo, not the supporting host side code. It was kept in arch/x86/kernel because configuring the irq for the PSP through the ACPI interface requires poking at bits from the architectural vector domain.

This series is a prerequisite for nested SNP-host support on Hyper-V but is independent of the SNP-host support patch set. Hyper-V only supports nested SEV-SNP (not SEV or SEV-ES) so the PSP only supports a subset of the full PSP command set. Without SNP-host support (which is not upstream yet), the only PSP command that will succeed is SEV_PLATFORM_STATUS.

Changes since v1:
* move platform_device_add_data() call to commit that introduces psp device
* change psp dependency from CONFIG_AMD_MEM_ENCRYPT to CONFIG_KVM_AMD_SEV
* add blank lines, s/plat/platform/, remove variable initializers before first use, remove masking/shifting where not needed
* dynamically allocate sev_vdata/psp_vdata structs instead of overwriting static variables

Jeremi Piotrowski (8):
include/acpi: add definition of ASPT table
ACPI: ASPT: Add helper to parse table
x86/psp: Register PSP platform device when ASP table is present x86/psp: Add IRQ support
crypto: cpp - Bind to psp platform device on x86
crypto: ccp - Add vdata for platform device
crypto: ccp - Skip DMA coherency check for platform psp
crypto: ccp - Allow platform device to be psp master device

 arch/x86/kernel/Makefile          |   1 +
 arch/x86/kernel/psp.c             | 219 ++++++++++++++++++++++++++++++
 drivers/acpi/Makefile             |   1 +
 drivers/acpi/aspt.c               | 104 ++++++++++++++
 drivers/crypto/ccp/sp-dev.c       |  66 +++++++++
 drivers/crypto/ccp/sp-dev.h       |   4 +
 drivers/crypto/ccp/sp-pci.c       |  48 -------
 drivers/crypto/ccp/sp-platform.c  |  76 ++++++++++-
 include/acpi/actbl1.h             |  46 +++++++
 include/linux/platform_data/psp.h |  32 +++++
 10 files changed, 548 insertions(+), 49 deletions(-)
 create mode 100644 arch/x86/kernel/psp.c
 create mode 100644 drivers/acpi/aspt.c
 create mode 100644 include/linux/platform_data/psp.h
Signature · 1 line
2.25.1
#1 lore raw

PATCH v2 1/8 include/acpi: add definition of ASPT table

The AMD Secure Processor ACPI Table provides the memory location of the register window and register offsets necessary to communicate with AMD's PSP (Platform Security Processor). This table is exposed on Hyper-V VMs configured with support for AMD's SNP isolation technology.

---
 include/acpi/actbl1.h | 46 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
include/acpi/actbl1.h +46 -0
index 15c78678c5d3..00d40373df37 100644
@@ -26,6 +26,7 @@
  */
 #define ACPI_SIG_AEST           "AEST"	/* Arm Error Source Table */
 #define ACPI_SIG_ASF            "ASF!"	/* Alert Standard Format table */
+#define ACPI_SIG_ASPT           "ASPT"	/* AMD Secure Processor Table */
 #define ACPI_SIG_BERT           "BERT"	/* Boot Error Record Table */
 #define ACPI_SIG_BGRT           "BGRT"	/* Boot Graphics Resource Table */
 #define ACPI_SIG_BOOT           "BOOT"	/* Simple Boot Flag Table */
@@ -106,6 +107,51 @@ struct acpi_whea_header {
 	u64 mask;		/* Bitmask required for this register instruction */
 };
 
+/* https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/acpitabl/ns-acpitabl-aspt_table */
+#define ASPT_REVISION_ID 0x01
+struct acpi_table_aspt {
+	struct acpi_table_header header;
+	u32 num_entries;
+};
+
+struct acpi_aspt_header {
+	u16 type;
+	u16 length;
+};
+
+enum acpi_aspt_type {
+	ACPI_ASPT_TYPE_GLOBAL_REGS = 0,
+	ACPI_ASPT_TYPE_SEV_MBOX_REGS = 1,
+	ACPI_ASPT_TYPE_ACPI_MBOX_REGS = 2,
+};
+
+/* 0: ASPT Global Registers */
+struct acpi_aspt_global_regs {
+	struct acpi_aspt_header header;
+	u32 reserved;
+	u64 feature_reg_addr;
+	u64 irq_en_reg_addr;
+	u64 irq_st_reg_addr;
+};
+
+/* 1: ASPT SEV Mailbox Registers */
+struct acpi_aspt_sev_mbox_regs {
+	struct acpi_aspt_header header;
+	u8 mbox_irq_id;
+	u8 reserved[3];
+	u64 cmd_resp_reg_addr;
+	u64 cmd_buf_lo_reg_addr;
+	u64 cmd_buf_hi_reg_addr;
+};
+
+/* 2: ASPT ACPI Mailbox Registers */
+struct acpi_aspt_acpi_mbox_regs {
+	struct acpi_aspt_header header;
+	u32 reserved1;
+	u64 cmd_resp_reg_addr;
+	u64 reserved2[2];
+};
+
 /*******************************************************************************
  *
  * ASF - Alert Standard Format table (Signature "ASF!")
Signature · 1 line
2.25.1
#2 lore raw

Re: [PATCH v2 1/8] include/acpi: add definition of ASPT table

Quoted 7 lines · Jeremi Piotrowski

The AMD Secure Processor ACPI Table provides the memory location of the register window and register offsets necessary to communicate with AMD's PSP (Platform Security Processor). This table is exposed on Hyper-V VMs configured with support for AMD's SNP isolation technology.

Please add a Link tag pointing to the corresponding upstream ACPICA pull request (or upstream ACPICA commit if already pulled) to this patch and analogously for patch [2/8].

Thanks!

Quoted 71 lines
---
 include/acpi/actbl1.h | 46 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
include/acpi/actbl1.h +46 -1
index 15c78678c5d3..00d40373df37 100644
@@ -26,6 +26,7 @@
  */
 #define ACPI_SIG_AEST           "AEST" /* Arm Error Source Table */
 #define ACPI_SIG_ASF            "ASF!" /* Alert Standard Format table */
+#define ACPI_SIG_ASPT           "ASPT" /* AMD Secure Processor Table */
 #define ACPI_SIG_BERT           "BERT" /* Boot Error Record Table */
 #define ACPI_SIG_BGRT           "BGRT" /* Boot Graphics Resource Table */
 #define ACPI_SIG_BOOT           "BOOT" /* Simple Boot Flag Table */
@@ -106,6 +107,51 @@ struct acpi_whea_header {
        u64 mask;               /* Bitmask required for this register instruction */
 };
 
+/* https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/acpitabl/ns-acpitabl-aspt_table */
+#define ASPT_REVISION_ID 0x01
+struct acpi_table_aspt {
+       struct acpi_table_header header;
+       u32 num_entries;
+};
+
+struct acpi_aspt_header {
+       u16 type;
+       u16 length;
+};
+
+enum acpi_aspt_type {
+       ACPI_ASPT_TYPE_GLOBAL_REGS = 0,
+       ACPI_ASPT_TYPE_SEV_MBOX_REGS = 1,
+       ACPI_ASPT_TYPE_ACPI_MBOX_REGS = 2,
+};
+
+/* 0: ASPT Global Registers */
+struct acpi_aspt_global_regs {
+       struct acpi_aspt_header header;
+       u32 reserved;
+       u64 feature_reg_addr;
+       u64 irq_en_reg_addr;
+       u64 irq_st_reg_addr;
+};
+
+/* 1: ASPT SEV Mailbox Registers */
+struct acpi_aspt_sev_mbox_regs {
+       struct acpi_aspt_header header;
+       u8 mbox_irq_id;
+       u8 reserved[3];
+       u64 cmd_resp_reg_addr;
+       u64 cmd_buf_lo_reg_addr;
+       u64 cmd_buf_hi_reg_addr;
+};
+
+/* 2: ASPT ACPI Mailbox Registers */
+struct acpi_aspt_acpi_mbox_regs {
+       struct acpi_aspt_header header;
+       u32 reserved1;
+       u64 cmd_resp_reg_addr;
+       u64 reserved2[2];
+};
+
 /*******************************************************************************
  *
  * ASF - Alert Standard Format table (Signature "ASF!")
--

2.25.1

#3 lore raw

Re: [PATCH v2 1/8] include/acpi: add definition of ASPT table

Quoted 8 lines · Rafael J. Wysocki
Quoted 6 lines · Jeremi Piotrowski

The AMD Secure Processor ACPI Table provides the memory location of the register window and register offsets necessary to communicate with AMD's PSP (Platform Security Processor). This table is exposed on Hyper-V VMs configured with support for AMD's SNP isolation technology.

Quoted 8 lines
Quoted 1 line

Please add a Link tag pointing to the corresponding upstream ACPICA pull request (or upstream ACPICA commit if already pulled) to this patch and analogously for patch [2/8].

Thanks!

Will do when I post v3. I'll poke the ACPICA maintainer again after the specification gets published this friday.

Jeremi

Quoted 71 lines
Quoted 71 lines
---
 include/acpi/actbl1.h | 46 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
include/acpi/actbl1.h +46 -1
index 15c78678c5d3..00d40373df37 100644
@@ -26,6 +26,7 @@
  */
 #define ACPI_SIG_AEST           "AEST" /* Arm Error Source Table */
 #define ACPI_SIG_ASF            "ASF!" /* Alert Standard Format table */
+#define ACPI_SIG_ASPT           "ASPT" /* AMD Secure Processor Table */
 #define ACPI_SIG_BERT           "BERT" /* Boot Error Record Table */
 #define ACPI_SIG_BGRT           "BGRT" /* Boot Graphics Resource Table */
 #define ACPI_SIG_BOOT           "BOOT" /* Simple Boot Flag Table */
@@ -106,6 +107,51 @@ struct acpi_whea_header {
        u64 mask;               /* Bitmask required for this register instruction */
 };
 
+/* https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/acpitabl/ns-acpitabl-aspt_table */
+#define ASPT_REVISION_ID 0x01
+struct acpi_table_aspt {
+       struct acpi_table_header header;
+       u32 num_entries;
+};
+
+struct acpi_aspt_header {
+       u16 type;
+       u16 length;
+};
+
+enum acpi_aspt_type {
+       ACPI_ASPT_TYPE_GLOBAL_REGS = 0,
+       ACPI_ASPT_TYPE_SEV_MBOX_REGS = 1,
+       ACPI_ASPT_TYPE_ACPI_MBOX_REGS = 2,
+};
+
+/* 0: ASPT Global Registers */
+struct acpi_aspt_global_regs {
+       struct acpi_aspt_header header;
+       u32 reserved;
+       u64 feature_reg_addr;
+       u64 irq_en_reg_addr;
+       u64 irq_st_reg_addr;
+};
+
+/* 1: ASPT SEV Mailbox Registers */
+struct acpi_aspt_sev_mbox_regs {
+       struct acpi_aspt_header header;
+       u8 mbox_irq_id;
+       u8 reserved[3];
+       u64 cmd_resp_reg_addr;
+       u64 cmd_buf_lo_reg_addr;
+       u64 cmd_buf_hi_reg_addr;
+};
+
+/* 2: ASPT ACPI Mailbox Registers */
+struct acpi_aspt_acpi_mbox_regs {
+       struct acpi_aspt_header header;
+       u32 reserved1;
+       u64 cmd_resp_reg_addr;
+       u64 reserved2[2];
+};
+
 /*******************************************************************************
  *
  * ASF - Alert Standard Format table (Signature "ASF!")
--

2.25.1

#4 lore raw

PATCH v2 2/8 ACPI: ASPT: Add helper to parse table

In reply to Jeremi Piotrowski

The ASP table indicates the presence of a Platform Security Processor with a register window and registers to configure interrupt delivery. The helper checks for the presence of the table and returns a resource and struct with register offsets.

---
 drivers/acpi/Makefile             |   1 +
 drivers/acpi/aspt.c               | 104 ++++++++++++++++++++++++++++++
 include/linux/platform_data/psp.h |  32 +++++++++
 3 files changed, 137 insertions(+)
 create mode 100644 drivers/acpi/aspt.c
 create mode 100644 include/linux/platform_data/psp.h
drivers/acpi/Makefile +1 -0
index 0002eecbf870..9621c90e0221 100644
@@ -57,6 +57,7 @@ acpi-y				+= evged.o
 acpi-y				+= sysfs.o
 acpi-y				+= property.o
 acpi-$(CONFIG_X86)		+= acpi_cmos_rtc.o
+acpi-$(CONFIG_X86)		+= aspt.o
 acpi-$(CONFIG_X86)		+= x86/apple.o
 acpi-$(CONFIG_X86)		+= x86/utils.o
 acpi-$(CONFIG_X86)		+= x86/s2idle.o
drivers/acpi/aspt.c new file +104 -0

104 lines not on this page · raw message

include/linux/platform_data/psp.h new file +32 -0
new file mode 100644
index 000000000000..b761f72168d6
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * psp.h - PSP register offsets parsed from ASPT ACPI table
+ */
+
+#ifndef __LINUX_PSP_H
+#define __LINUX_PSP_H
+
+#include <linux/types.h>
+#include <linux/ioport.h>
+
+struct psp_platform_data {
+	int sev_cmd_resp_reg;
+	int sev_cmd_buf_lo_reg;
+	int sev_cmd_buf_hi_reg;
+	int feature_reg;
+	int irq_en_reg;
+	int irq_st_reg;
+	int mbox_irq_id;
+	int acpi_cmd_resp_reg;
+};
+
+#if IS_ENABLED(CONFIG_ACPI)
+int acpi_parse_aspt(struct resource *res, struct psp_platform_data *pdata);
+#else
+static inline acpi_parse_aspt(struct resource *res, struct psp_platform_data *pdata)
+{
+	return -ENODEV;
+}
+#endif
+
+#endif /* __LINUX_PSP_H */
Signature · 1 line
2.25.1
#5 lore raw

PATCH v2 3/8 x86/psp: Register PSP platform device when ASP table is present

In reply to Jeremi Piotrowski

The ASP table contains the memory location of the register window for communication with the Platform Security Processor. The device is not exposed as an acpi node, so it is necessary to probe for the table and register a platform_device to represent it in the kernel.

At least conceptually, the same PSP may be exposed on the PCIe bus as well, in which case it would be necessary to choose whether to use a PCI BAR or the register window defined in ASPT for communication. There is no advantage to using the ACPI and there are no known bare-metal systems that expose the ASP table, so device registration is restricted to the only systems known to provide an ASPT: Hyper-V VMs. Hyper-V VMs also do not expose the PSP over PCIe.

This is a skeleton device at this point, as the ccp driver is not yet prepared to correctly probe it. Interrupt configuration will come later on as well.

---
 arch/x86/kernel/Makefile |  1 +
 arch/x86/kernel/psp.c    | 42 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)
 create mode 100644 arch/x86/kernel/psp.c
arch/x86/kernel/Makefile +1 -0
index f901658d9f7c..f7b71aa245e8 100644
@@ -139,6 +139,7 @@ obj-$(CONFIG_UNWINDER_ORC)		+= unwind_orc.o
 obj-$(CONFIG_UNWINDER_FRAME_POINTER)	+= unwind_frame.o
 obj-$(CONFIG_UNWINDER_GUESS)		+= unwind_guess.o
 
+obj-$(CONFIG_KVM_AMD_SEV)		+= psp.o
 obj-$(CONFIG_AMD_MEM_ENCRYPT)		+= sev.o
 
 obj-$(CONFIG_CFI_CLANG)			+= cfi.o
arch/x86/kernel/psp.c new file +42 -0

42 lines not on this page · raw message

Signature · 1 line
2.25.1
#6 lore raw

PATCH v2 4/8 x86/psp: Add IRQ support

In reply to Jeremi Piotrowski

The ACPI PSP device provides a mailbox irq that needs to be configured through the ACPI mailbox register first. This requires passing a CPU vector and physical CPU id and then enabling interrupt delivery. Allocate the irq directly from the default irq domain (x86_vector_domain) to get access to the required information. By passing a cpumask through irq_alloc_info the vector is immediately allocated (and not later during activation) and can be retrieved.

---
 arch/x86/kernel/psp.c | 185 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 181 insertions(+), 4 deletions(-)
arch/x86/kernel/psp.c +181 -4
index 64f3bfc5c9ff..fc059cf3b25c 100644
@@ -1,8 +1,182 @@
 // SPDX-License-Identifier: GPL-2.0-only
-
+#define pr_fmt(fmt) "psp: " fmt
 #include <linux/platform_data/psp.h>
 #include <linux/platform_device.h>
+#include <linux/iopoll.h>
+#include <linux/irq.h>
 #include <asm/hypervisor.h>
+#include <asm/irqdomain.h>
+
+#define PSP_ACPI_CMDID_SHIFT 16
+#define PSP_ACPI_STATUS_SHIFT 26
+#define PSP_ACPI_STATUS_MASK GENMASK(30, 26)
+#define PSP_ACPI_RESPONSE_BIT BIT(31)
+#define PSP_ACPI_VECTOR_MASK GENMASK(7, 0)
+#define PSP_ACPI_MBOX_IRQID_SHIFT 10
+#define PSP_ACPI_IRQ_EN_BIT BIT(0)
+#define PSP_ACPI_IRQ_EN_MBOX_IRQID_SHIFT 10
+
+#define PSP_CMD_DELAY_US 2
+#define PSP_CMD_TIMEOUT_US 10000
+
+enum ASP_CMDID {
+	ASP_CMDID_PART1  = 0x82,
+	ASP_CMDID_PART2  = 0x83,
+	ASP_CMDID_PART3  = 0x84,
+	ASP_CMDID_IRQ_EN = 0x85,
+};
+
+enum ASP_CMD_STATUS {
+	ASP_CMD_STATUS_SUCCESS = 0x0,
+	ASP_CMD_STATUS_INVALID_CMD = 0x1,
+	ASP_CMD_STATUS_INVALID_PARAM = 0x2,
+	ASP_CMD_STATUS_INVALID_FW_STATE = 0x3,
+	ASP_CMD_STATUS_FAILURE = 0x1F,
+};
+
+struct psp_irq_data {
+	void __iomem *base;
+	u8 mbox_irq_id;
+	int acpi_cmd_resp_reg;
+};
+
+static int psp_sync_cmd(void __iomem *reg, u8 cmd, u16 data)
+{
+	u32 val;
+	int err;
+
+	val  = data;
+	val |= cmd << PSP_ACPI_CMDID_SHIFT;
+	writel(val, reg);
+	err = readl_poll_timeout_atomic(reg, val, val & PSP_ACPI_RESPONSE_BIT, PSP_CMD_DELAY_US,
+					PSP_CMD_TIMEOUT_US);
+	if (err)
+		return err;
+
+	return (val & PSP_ACPI_STATUS_MASK) >> PSP_ACPI_STATUS_SHIFT;
+}
+
+static int psp_set_irq_enable(struct psp_irq_data *data, bool irq_en)
+{
+	void __iomem *reg = data->base + data->acpi_cmd_resp_reg;
+	u16 val = 0;
+	int err;
+
+	if (data->mbox_irq_id > 63)
+		return -EINVAL;
+
+	val  = irq_en ? PSP_ACPI_IRQ_EN_BIT : 0;
+	val |= data->mbox_irq_id << PSP_ACPI_IRQ_EN_MBOX_IRQID_SHIFT;
+	err = psp_sync_cmd(reg, ASP_CMDID_IRQ_EN, val);
+	if (err != ASP_CMD_STATUS_SUCCESS) {
+		pr_err("ASP_CMDID_IRQ_EN failed: %d\n", err);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int psp_configure_irq(struct psp_irq_data *data, unsigned int vector, unsigned int cpu)
+{
+	void __iomem *reg = data->base + data->acpi_cmd_resp_reg;
+	unsigned int dest_cpu = cpu_physical_id(cpu);
+	u16 part1, part2, part3;
+	int err;
+
+	if (data->mbox_irq_id > 63)
+		return -EINVAL;
+
+	part1  = dest_cpu;
+	part2  = dest_cpu >> 16;
+	part3  = vector & PSP_ACPI_VECTOR_MASK;
+	part3 |= data->mbox_irq_id << PSP_ACPI_MBOX_IRQID_SHIFT;
+
+	err = psp_sync_cmd(reg, ASP_CMDID_PART1, part1);
+	if (err != ASP_CMD_STATUS_SUCCESS) {
+		pr_err("ASP_CMDID_PART1 failed: %d\n", err);
+		return -EIO;
+	}
+	err = psp_sync_cmd(reg, ASP_CMDID_PART2, part2);
+	if (err != ASP_CMD_STATUS_SUCCESS) {
+		pr_err("ASP_CMDID_PART2 failed: %d\n", err);
+		return -EIO;
+	}
+	err = psp_sync_cmd(reg, ASP_CMDID_PART3, part3);
+	if (err != ASP_CMD_STATUS_SUCCESS) {
+		pr_err("ASP_CMDID_PART3 failed: %d\n", err);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int psp_init_irq(const struct psp_platform_data *pdata, const struct resource *reg,
+			struct resource *irq)
+{
+	struct psp_irq_data pspirqd;
+	struct irq_alloc_info info;
+	struct irq_data *data;
+	struct irq_cfg *cfg;
+	void __iomem *base;
+	int virq;
+	int err;
+
+	base = ioremap(reg->start, resource_size(reg));
+	if (!base)
+		return -ENOMEM;
+
+	pspirqd.mbox_irq_id = pdata->mbox_irq_id;
+	pspirqd.acpi_cmd_resp_reg = pdata->acpi_cmd_resp_reg;
+	pspirqd.base = base;
+	init_irq_alloc_info(&info, cpumask_of(0));
+	virq = irq_domain_alloc_irqs(NULL, 1, NUMA_NO_NODE, &info);
+	if (virq <= 0) {
+		pr_err("failed to allocate vector: %d\n", virq);
+		err = -ENOMEM;
+		goto unmap;
+	}
+	irq_set_handler(virq, handle_edge_irq);
+
+	data = irq_get_irq_data(virq);
+	if (!data) {
+		pr_err("no irq data\n");
+		err = -ENODEV;
+		goto freeirq;
+	}
+
+	cfg = irqd_cfg(data);
+	if (!cfg) {
+		pr_err("no irq cfg\n");
+		err = -ENODEV;
+		goto freeirq;
+	}
+
+	err = psp_configure_irq(&pspirqd, cfg->vector, 0);
+	if (err) {
+		pr_err("failed to configure irq: %d\n", err);
+		goto freeirq;
+	}
+
+	err = psp_set_irq_enable(&pspirqd, true);
+	if (err) {
+		pr_err("failed to enable irq: %d\n", err);
+		goto freeirq;
+	}
+
+	*irq = (struct resource)DEFINE_RES_IRQ(virq);
+
+	iounmap(base);
+
+	return 0;
+
+freeirq:
+	irq_domain_free_irqs(virq, 1);
+
+unmap:
+	iounmap(base);
+
+	return err;
+}
 
 static struct platform_device psp_device = {
 	.name           = "psp",
@@ -12,7 +186,7 @@ static struct platform_device psp_device = {
 static int __init psp_init_platform_device(void)
 {
 	struct psp_platform_data pdata = {};
-	struct resource res[1];
+	struct resource res[2];
 	int err;
 
 	/*
@@ -24,10 +198,13 @@ static int __init psp_init_platform_device(void)
 	if (!hypervisor_is_type(X86_HYPER_MS_HYPERV))
 		return -ENODEV;
 
-	err = acpi_parse_aspt(res, &pdata);
+	err = acpi_parse_aspt(&res[0], &pdata);
+	if (err)
+		return err;
+	err = psp_init_irq(&pdata, &res[0], &res[1]);
 	if (err)
 		return err;
-	err = platform_device_add_resources(&psp_device, res, 1);
+	err = platform_device_add_resources(&psp_device, res, 2);
 	if (err)
 		return err;
 	err = platform_device_add_data(&psp_device, &pdata, sizeof(pdata));
Signature · 1 line
2.25.1
#7 lore raw

PATCH v2 5/8 crypto: cpp - Bind to psp platform device on x86

In reply to Jeremi Piotrowski

The PSP in Hyper-V VMs is exposed through the ASP ACPI table and is represented as a platform_device. Allow the ccp driver to bind to it by adding an id_table and initing the platform_driver also on x86. At this point probe is called for the psp device but init fails due to missing driver data.

---
 drivers/crypto/ccp/sp-dev.c      | 7 +++++++
 drivers/crypto/ccp/sp-platform.c | 7 +++++++
 2 files changed, 14 insertions(+)
drivers/crypto/ccp/sp-dev.c +7 -0
index 7eb3e4668286..4c9f442b8a11 100644
@@ -259,6 +259,12 @@ static int __init sp_mod_init(void)
 	if (ret)
 		return ret;
 
+	ret = sp_platform_init();
+	if (ret) {
+		sp_pci_exit();
+		return ret;
+	}
+
 #ifdef CONFIG_CRYPTO_DEV_SP_PSP
 	psp_pci_init();
 #endif
@@ -286,6 +292,7 @@ static void __exit sp_mod_exit(void)
 #ifdef CONFIG_CRYPTO_DEV_SP_PSP
 	psp_pci_exit();
 #endif
+	sp_platform_exit();
 
 	sp_pci_exit();
 #endif
drivers/crypto/ccp/sp-platform.c +7 -0
index 7d79a8744f9a..5dcc834deb72 100644
@@ -56,6 +56,12 @@ static const struct of_device_id sp_of_match[] = {
 MODULE_DEVICE_TABLE(of, sp_of_match);
 #endif
 
+static const struct platform_device_id sp_platform_match[] = {
+	{ "psp" },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, sp_platform_match);
+
 static struct sp_dev_vdata *sp_get_of_version(struct platform_device *pdev)
 {
 #ifdef CONFIG_OF
@@ -212,6 +218,7 @@ static int sp_platform_resume(struct platform_device *pdev)
 #endif
 
 static struct platform_driver sp_platform_driver = {
+	.id_table = sp_platform_match,
 	.driver = {
 		.name = "ccp",
 #ifdef CONFIG_ACPI
Signature · 1 line
2.25.1
#8 lore raw

PATCH v2 6/8 crypto: ccp - Add vdata for platform device

In reply to Jeremi Piotrowski

When matching the "psp" platform_device, the register offsets are determined at runtime from the ASP ACPI table. The structure containing the offset is passed through the platform data field provided before registering the platform device.

To support this scenario, dynamically allocate vdata structs and fill those in with offsets provided by the platform. Due to the fields of the structs being const, it was necessary to use temporary structs and memcpy, as any assignment of the whole struct fails with an 'read-only location' compiler error.

---
 drivers/crypto/ccp/sp-platform.c | 57 ++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)
drivers/crypto/ccp/sp-platform.c +57 -0
index 5dcc834deb72..2e57ec15046b 100644
@@ -22,6 +22,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/acpi.h>
+#include <linux/platform_data/psp.h>
 
 #include "ccp-dev.h"
 
@@ -86,6 +87,60 @@ static struct sp_dev_vdata *sp_get_acpi_version(struct platform_device *pdev)
 	return NULL;
 }
 
+static void sp_platform_fill_vdata(struct sp_dev_vdata *vdata, struct psp_vdata *psp,
+				   struct sev_vdata *sev, const struct psp_platform_data *pdata)
+{
+	struct sev_vdata sevtmp = {
+		.cmdresp_reg = pdata->sev_cmd_resp_reg,
+		.cmdbuff_addr_lo_reg = pdata->sev_cmd_buf_lo_reg,
+		.cmdbuff_addr_hi_reg = pdata->sev_cmd_buf_hi_reg,
+	};
+	struct psp_vdata psptmp = {
+		.sev = sev,
+		.feature_reg = pdata->feature_reg,
+		.inten_reg = pdata->irq_en_reg,
+		.intsts_reg = pdata->irq_st_reg,
+	};
+
+	memcpy(sev, &sevtmp, sizeof(*sev));
+	memcpy(psp, &psptmp, sizeof(*psp));
+	vdata->psp_vdata = psp;
+}
+
+static struct sp_dev_vdata *sp_get_platform_version(struct sp_device *sp)
+{
+	struct psp_platform_data *pdata;
+	struct device *dev = sp->dev;
+	struct sp_dev_vdata *vdata;
+	struct psp_vdata *psp;
+	struct sev_vdata *sev;
+
+	pdata = dev_get_platdata(dev);
+	if (!pdata) {
+		dev_err(dev, "missing platform data\n");
+		return NULL;
+	}
+
+	vdata = devm_kzalloc(dev, sizeof(*vdata) + sizeof(*psp) + sizeof(*sev), GFP_KERNEL);
+	if (!vdata)
+		return NULL;
+
+	psp = (void *)vdata + sizeof(*vdata);
+	sev = (void *)psp + sizeof(*psp);
+	sp_platform_fill_vdata(vdata, psp, sev, pdata);
+
+	dev_dbg(dev, "PSP feature register:\t%x\n", psp->feature_reg);
+	dev_dbg(dev, "PSP IRQ enable register:\t%x\n", psp->inten_reg);
+	dev_dbg(dev, "PSP IRQ status register:\t%x\n", psp->intsts_reg);
+	dev_dbg(dev, "SEV cmdresp register:\t%x\n", sev->cmdresp_reg);
+	dev_dbg(dev, "SEV cmdbuf lo register:\t%x\n", sev->cmdbuff_addr_lo_reg);
+	dev_dbg(dev, "SEV cmdbuf hi register:\t%x\n", sev->cmdbuff_addr_hi_reg);
+	dev_dbg(dev, "SEV cmdresp IRQ:\t%x\n", pdata->mbox_irq_id);
+	dev_dbg(dev, "ACPI cmdresp register:\t%x\n", pdata->acpi_cmd_resp_reg);
+
+	return vdata;
+}
+
 static int sp_get_irqs(struct sp_device *sp)
 {
 	struct sp_platform *sp_platform = sp->dev_specific;
@@ -137,6 +192,8 @@ static int sp_platform_probe(struct platform_device *pdev)
 	sp->dev_specific = sp_platform;
 	sp->dev_vdata = pdev->dev.of_node ? sp_get_of_version(pdev)
 					 : sp_get_acpi_version(pdev);
+	if (!sp->dev_vdata)
+		sp->dev_vdata = sp_get_platform_version(sp);
 	if (!sp->dev_vdata) {
 		ret = -ENODEV;
 		dev_err(dev, "missing driver data\n");
Signature · 1 line
2.25.1
#9 lore raw

PATCH v2 7/8 crypto: ccp - Skip DMA coherency check for platform psp

In reply to Jeremi Piotrowski

The value of device_get_dma_attr() is only relevenat for ARM64 and CCP devices to configure the value of the axcache attribute used to access memory by the coprocessor. None of this applies to the platform psp so skip it. Skip the dma_attr check by keeping track of the fact that we are a pure platform device.

---
 drivers/crypto/ccp/sp-platform.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/crypto/ccp/sp-platform.c +5 -1
index 2e57ec15046b..be8306c47196 100644
@@ -29,6 +29,7 @@
 struct sp_platform {
 	int coherent;
 	unsigned int irq_count;
+	bool is_platform_device;
 };
 
 static const struct sp_dev_vdata dev_vdata[] = {
@@ -109,6 +110,7 @@ static void sp_platform_fill_vdata(struct sp_dev_vdata *vdata, struct psp_vdata
 
 static struct sp_dev_vdata *sp_get_platform_version(struct sp_device *sp)
 {
+	struct sp_platform *sp_platform = sp->dev_specific;
 	struct psp_platform_data *pdata;
 	struct device *dev = sp->dev;
 	struct sp_dev_vdata *vdata;
@@ -129,6 +131,8 @@ static struct sp_dev_vdata *sp_get_platform_version(struct sp_device *sp)
 	sev = (void *)psp + sizeof(*psp);
 	sp_platform_fill_vdata(vdata, psp, sev, pdata);
 
+	sp_platform->is_platform_device = true;
+
 	dev_dbg(dev, "PSP feature register:\t%x\n", psp->feature_reg);
 	dev_dbg(dev, "PSP IRQ enable register:\t%x\n", psp->inten_reg);
 	dev_dbg(dev, "PSP IRQ status register:\t%x\n", psp->intsts_reg);
@@ -207,7 +211,7 @@ static int sp_platform_probe(struct platform_device *pdev)
 	}
 
 	attr = device_get_dma_attr(dev);
-	if (attr == DEV_DMA_NOT_SUPPORTED) {
+	if (attr == DEV_DMA_NOT_SUPPORTED && !sp_platform->is_platform_device) {
 		dev_err(dev, "DMA is not supported");
 		goto e_err;
 	}
Signature · 1 line
2.25.1
#10 lore raw

PATCH v2 8/8 crypto: ccp - Allow platform device to be psp master device

In reply to Jeremi Piotrowski

Move the getters/setters to sp-dev.c, so that they can be accessed from sp-pci.c and sp-platform.c. This makes it possible for the psp platform_device to set the function pointers and be assigned the role of master device by psp_dev_init().

While the case of a system having both a PCI and ACPI PSP is not supported (and not known to occur in the wild), it makes sense to have a single static global to assign to. Should such a system occur, the logic in psp_set_master() is that the pci device is preferred.

---
 drivers/crypto/ccp/sp-dev.c      | 59 ++++++++++++++++++++++++++++++++
 drivers/crypto/ccp/sp-dev.h      |  4 +++
 drivers/crypto/ccp/sp-pci.c      | 48 --------------------------
 drivers/crypto/ccp/sp-platform.c |  6 ++++
 4 files changed, 69 insertions(+), 48 deletions(-)
drivers/crypto/ccp/sp-dev.c +58 -0
index 4c9f442b8a11..71461dfdfa4f 100644
@@ -14,6 +14,8 @@
 #include <linux/kthread.h>
 #include <linux/sched.h>
 #include <linux/interrupt.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
 #include <linux/spinlock.h>
 #include <linux/spinlock_types.h>
 #include <linux/types.h>
@@ -39,6 +41,8 @@ static LIST_HEAD(sp_units);
 /* Ever-increasing value to produce unique unit numbers */
 static atomic_t sp_ordinal;
 
+static struct sp_device *sp_dev_master;
+
 static void sp_add_device(struct sp_device *sp)
 {
 	unsigned long flags;
@@ -250,6 +254,60 @@ struct sp_device *sp_get_psp_master_device(void)
 	return ret;
 }
 
+static bool sp_pci_is_master(struct sp_device *sp)
+{
+	struct device *dev_cur, *dev_new;
+	struct pci_dev *pdev_cur, *pdev_new;
+
+	dev_new = sp->dev;
+	dev_cur = sp_dev_master->dev;
+
+	pdev_new = to_pci_dev(dev_new);
+	pdev_cur = to_pci_dev(dev_cur);
+
+	if (pdev_new->bus->number < pdev_cur->bus->number)
+		return true;
+
+	if (PCI_SLOT(pdev_new->devfn) < PCI_SLOT(pdev_cur->devfn))
+		return true;
+
+	if (PCI_FUNC(pdev_new->devfn) < PCI_FUNC(pdev_cur->devfn))
+		return true;
+
+	return false;
+}
+
+void psp_set_master(struct sp_device *sp)
+{
+	struct device *dev_cur, *dev_new;
+
+	if (!sp_dev_master) {
+		sp_dev_master = sp;
+		return;
+	}
+
+	dev_new = sp->dev;
+	dev_cur = sp_dev_master->dev;
+
+	if (dev_is_pci(dev_new) && dev_is_pci(dev_cur) && sp_pci_is_master(sp))
+		sp_dev_master = sp;
+	if (dev_is_pci(dev_new) && dev_is_platform(dev_cur))
+		sp_dev_master = sp;
+}
+
+struct sp_device *psp_get_master(void)
+{
+	return sp_dev_master;
+}
+
+void psp_clear_master(struct sp_device *sp)
+{
+	if (sp == sp_dev_master) {
+		sp_dev_master = NULL;
+		dev_dbg(sp->dev, "Cleared sp_dev_master\n");
+	}
+}
+
 static int __init sp_mod_init(void)
 {
 #ifdef CONFIG_X86
drivers/crypto/ccp/sp-dev.h +4 -0
index 20377e67f65d..c05f1fa82ff4 100644
@@ -129,6 +129,10 @@ int sp_request_psp_irq(struct sp_device *sp, irq_handler_t handler,
 void sp_free_psp_irq(struct sp_device *sp, void *data);
 struct sp_device *sp_get_psp_master_device(void);
 
+void psp_set_master(struct sp_device *sp);
+struct sp_device *psp_get_master(void);
+void psp_clear_master(struct sp_device *sp);
+
 #ifdef CONFIG_CRYPTO_DEV_SP_CCP
 
 int ccp_dev_init(struct sp_device *sp);
drivers/crypto/ccp/sp-pci.c +0 -48

60 lines not on this page · raw message

drivers/crypto/ccp/sp-platform.c +6 -0
index be8306c47196..e22d9fee0956 100644
@@ -234,6 +234,12 @@ static int sp_platform_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(dev, sp);
 
+	if (sp_platform->is_platform_device) {
+		sp->set_psp_master_device = psp_set_master;
+		sp->get_psp_master_device = psp_get_master;
+		sp->clear_psp_master_device = psp_clear_master;
+	}
+
 	ret = sp_init(sp);
 	if (ret)
 		goto e_err;
Signature · 1 line
2.25.1
maintainer Acked-by on the v2 series
#11 lore raw
In reply to Jeremi Piotrowski
Quoted 12 lines · Jeremi Piotrowski

This patch series introduces support for discovering AMD's PSP from an ACPI table and extends the CCP driver to allow binding to that device on x86. This method of PSP discovery is used on Hyper-V when SNP isolation support is exposed to the guest. There is no ACPI node associated with this PSP, so after parsing the ASPT it is registered with the system as a platform_device.

I thought about putting psp.c in arch/x86/coco, but that directory is meant for the (confidential) guest side of CoCo, not the supporting host side code. It was kept in arch/x86/kernel because configuring the irq for the PSP through the ACPI interface requires poking at bits from the architectural vector domain.

Quoted 6 lines

This series is a prerequisite for nested SNP-host support on Hyper-V but is independent of the SNP-host support patch set. Hyper-V only supports nested SEV-SNP (not SEV or SEV-ES) so the PSP only supports a subset of the full PSP command set. Without SNP-host support (which is not upstream yet), the only PSP command that will succeed is SEV_PLATFORM_STATUS.

For the series:

Probably want Boris to weigh in on whether he wants the new psp.c file located in arch/x86/kernel, though.

Quoted 33 lines

Changes since v1:
* move platform_device_add_data() call to commit that introduces psp device
* change psp dependency from CONFIG_AMD_MEM_ENCRYPT to CONFIG_KVM_AMD_SEV
* add blank lines, s/plat/platform/, remove variable initializers before first use, remove masking/shifting where not needed
* dynamically allocate sev_vdata/psp_vdata structs instead of overwriting static variables

Jeremi Piotrowski (8):
include/acpi: add definition of ASPT table
ACPI: ASPT: Add helper to parse table
x86/psp: Register PSP platform device when ASP table is present x86/psp: Add IRQ support
crypto: cpp - Bind to psp platform device on x86
crypto: ccp - Add vdata for platform device
crypto: ccp - Skip DMA coherency check for platform psp
crypto: ccp - Allow platform device to be psp master device

  arch/x86/kernel/Makefile          |   1 +
  arch/x86/kernel/psp.c             | 219 ++++++++++++++++++++++++++++++
  drivers/acpi/Makefile             |   1 +
  drivers/acpi/aspt.c               | 104 ++++++++++++++
  drivers/crypto/ccp/sp-dev.c       |  66 +++++++++
  drivers/crypto/ccp/sp-dev.h       |   4 +
  drivers/crypto/ccp/sp-pci.c       |  48 -------
  drivers/crypto/ccp/sp-platform.c  |  76 ++++++++++-
  include/acpi/actbl1.h             |  46 +++++++
  include/linux/platform_data/psp.h |  32 +++++
  10 files changed, 548 insertions(+), 49 deletions(-)

create mode 100644 arch/x86/kernel/psp.c create mode 100644 drivers/acpi/aspt.c create mode 100644 include/linux/platform_data/psp.h