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
--
2.25.1
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.
Signed-off-by: Jeremi Piotrowski <[email protected]>
---
drivers/crypto/ccp/sp-platform.c | 57 ++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
index 5dcc834deb72..2e57ec15046b 100644
--- a/drivers/crypto/ccp/sp-platform.c
+++ b/drivers/crypto/ccp/sp-platform.c
@@ -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");
--
2.25.1
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.
Signed-off-by: Jeremi Piotrowski <[email protected]>
---
drivers/crypto/ccp/sp-platform.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
index 2e57ec15046b..be8306c47196 100644
--- a/drivers/crypto/ccp/sp-platform.c
+++ b/drivers/crypto/ccp/sp-platform.c
@@ -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;
}
--
2.25.1
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.
Signed-off-by: Jeremi Piotrowski <[email protected]>
---
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(-)
diff --git a/drivers/crypto/ccp/sp-dev.c b/drivers/crypto/ccp/sp-dev.c
index 4c9f442b8a11..71461dfdfa4f 100644
--- a/drivers/crypto/ccp/sp-dev.c
+++ b/drivers/crypto/ccp/sp-dev.c
@@ -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
diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h
index 20377e67f65d..c05f1fa82ff4 100644
--- a/drivers/crypto/ccp/sp-dev.h
+++ b/drivers/crypto/ccp/sp-dev.h
@@ -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);
diff --git a/drivers/crypto/ccp/sp-pci.c b/drivers/crypto/ccp/sp-pci.c
index 792d6da7f0c0..f9be8aba0acf 100644
--- a/drivers/crypto/ccp/sp-pci.c
+++ b/drivers/crypto/ccp/sp-pci.c
@@ -30,7 +30,6 @@ struct sp_pci {
int msix_count;
struct msix_entry msix_entry[MSIX_VECTORS];
};
-static struct sp_device *sp_dev_master;
#define attribute_show(name, def) \
static ssize_t name##_show(struct device *d, struct device_attribute *attr, \
@@ -168,53 +167,6 @@ static void sp_free_irqs(struct sp_device *sp)
sp->psp_irq = 0;
}
-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;
-}
-
-static void psp_set_master(struct sp_device *sp)
-{
- if (!sp_dev_master) {
- sp_dev_master = sp;
- return;
- }
-
- if (sp_pci_is_master(sp))
- sp_dev_master = sp;
-}
-
-static struct sp_device *psp_get_master(void)
-{
- return sp_dev_master;
-}
-
-static 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 sp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct sp_device *sp;
diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
index be8306c47196..e22d9fee0956 100644
--- a/drivers/crypto/ccp/sp-platform.c
+++ b/drivers/crypto/ccp/sp-platform.c
@@ -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;
--
2.25.1
On 2/13/23 03:24, Jeremi Piotrowski wrote:
> 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.
>
For the series:
Acked-by: Tom Lendacky <[email protected]>
Probably want Boris to weigh in on whether he wants the new psp.c file
located in arch/x86/kernel, though.
> 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
>