Re: [PATCH v1 6/8] crypto: ccp - Add vdata for platform device

From: Tom Lendacky
Date: Tue Jan 31 2023 - 15:36:15 EST


On 1/23/23 09:22, Jeremi Piotrowski wrote:
When matching the "psp" platform_device, determine the register offsets
at runtime from the ASP ACPI table. Pass the parsed register offsets
from the ASPT through platdata.

To support this scenario, mark the members of 'struct sev_vdata' and
'struct psp_vdata' non-const so that the probe function can write the
values. This does not affect the other users of sev_vdata/psp_vdata as
they define the whole struct const and the pointer in struct
sp_dev_vdata stays const too.

Signed-off-by: Jeremi Piotrowski <jpiotrowski@xxxxxxxxxxxxxxxxxxx>
---
arch/x86/kernel/psp.c | 3 ++
drivers/crypto/ccp/sp-dev.h | 12 +++----
drivers/crypto/ccp/sp-platform.c | 57 +++++++++++++++++++++++++++++++-
3 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/psp.c b/arch/x86/kernel/psp.c
index 24181d132bae..68511a14df63 100644
--- a/arch/x86/kernel/psp.c
+++ b/arch/x86/kernel/psp.c
@@ -199,6 +199,9 @@ static int __init psp_init_platform_device(void)
if (err)
return err;
err = platform_device_add_resources(&psp_device, res, 2);
+ if (err)
+ return err;
+ err = platform_device_add_data(&psp_device, &pdata, sizeof(pdata));
if (err)
return err;
diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h
index 20377e67f65d..aaa651364425 100644
--- a/drivers/crypto/ccp/sp-dev.h
+++ b/drivers/crypto/ccp/sp-dev.h
@@ -40,9 +40,9 @@ struct ccp_vdata {
};
struct sev_vdata {
- const unsigned int cmdresp_reg;
- const unsigned int cmdbuff_addr_lo_reg;
- const unsigned int cmdbuff_addr_hi_reg;
+ unsigned int cmdresp_reg;
+ unsigned int cmdbuff_addr_lo_reg;
+ unsigned int cmdbuff_addr_hi_reg;
};
struct tee_vdata {
@@ -56,9 +56,9 @@ struct tee_vdata {
struct psp_vdata {
const struct sev_vdata *sev;
const struct tee_vdata *tee;
- const unsigned int feature_reg;
- const unsigned int inten_reg;
- const unsigned int intsts_reg;
+ unsigned int feature_reg;
+ unsigned int inten_reg;
+ unsigned int intsts_reg;
};
/* Structure to hold SP device data */
diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
index ea8926e87981..281dbf6b150c 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"
@@ -30,11 +31,31 @@ struct sp_platform {
unsigned int irq_count;
};
+#ifdef CONFIG_CRYPTO_DEV_SP_PSP
+static struct sev_vdata sev_platform = {
+ .cmdresp_reg = -1,
+ .cmdbuff_addr_lo_reg = -1,
+ .cmdbuff_addr_hi_reg = -1,
+};
+static struct psp_vdata psp_platform = {
+ .sev = &sev_platform,
+ .feature_reg = -1,
+ .inten_reg = -1,
+ .intsts_reg = -1,
+};
+#endif
+
static const struct sp_dev_vdata dev_vdata[] = {
{
.bar = 0,
#ifdef CONFIG_CRYPTO_DEV_SP_CCP
.ccp_vdata = &ccpv3_platform,
+#endif
+ },
+ {
+ .bar = 0,
+#ifdef CONFIG_CRYPTO_DEV_SP_PSP
+ .psp_vdata = &psp_platform,
#endif
},
};
@@ -57,7 +78,7 @@ MODULE_DEVICE_TABLE(of, sp_of_match);
#endif
static const struct platform_device_id sp_plat_match[] = {
- { "psp" },
+ { "psp", (kernel_ulong_t)&dev_vdata[1] },
{ },
};
MODULE_DEVICE_TABLE(platform, sp_plat_match);
@@ -86,6 +107,38 @@ static struct sp_dev_vdata *sp_get_acpi_version(struct platform_device *pdev)
return NULL;
}
+static struct sp_dev_vdata *sp_get_plat_version(struct platform_device *pdev)
+{
+ struct sp_dev_vdata *drvdata = (struct sp_dev_vdata *)pdev->id_entry->driver_data;

s/drvdata/vdata/

+ struct device *dev = &pdev->dev;
+

Should check for null vdata and return NULL, e.g.:

if (!vdata)
return NULL;

+ if (drvdata == &dev_vdata[1]) {

This should be a check for vdata->psp_vdata being non-NULL and vdata->psp_vdata->sev being non-NULL, e.g.:

if (vdata->psp_vdata && vdata->psp_vdata->sev) {

+ struct psp_platform_data *pdata = dev_get_platdata(dev);
+
+ if (!pdata) {
+ dev_err(dev, "missing platform data\n");
+ return NULL;
+ }
+#ifdef CONFIG_CRYPTO_DEV_SP_PSP

No need for this with the above checks

+ psp_platform.feature_reg = pdata->feature_reg;

These should then be:

vdata->psp_vdata->inten_reg = pdata->feature_reg;
...

+ psp_platform.inten_reg = pdata->irq_en_reg;
+ psp_platform.intsts_reg = pdata->irq_st_reg;
+ sev_platform.cmdresp_reg = pdata->sev_cmd_resp_reg;

And this should be:

vdata->psp_vdata->sev->cmdbuff_addr_lo = ...

+ sev_platform.cmdbuff_addr_lo_reg = pdata->sev_cmd_buf_lo_reg;
+ sev_platform.cmdbuff_addr_hi_reg = pdata->sev_cmd_buf_hi_reg;
+ dev_dbg(dev, "GLBL feature:\t%x\n", pdata->feature_reg);

s/GLBL feature/PSP feature register/

+ dev_dbg(dev, "GLBL irq en:\t%x\n", pdata->irq_en_reg);

s/GLBL irq en/PSP IRQ enable register/

+ dev_dbg(dev, "GLBL irq st:\t%x\n", pdata->irq_st_reg);

s/GLBL irq st/PSP IRQ status register/

+ dev_dbg(dev, "SEV cmdresp:\t%x\n", pdata->sev_cmd_resp_reg);

s/SEV cmdresp/SEV cmdresp register/

+ dev_dbg(dev, "SEV cmdbuf lo:\t%x\n", pdata->sev_cmd_buf_lo_reg);

s/SEV cmdbuf lo/SEV cmdbuf lo register/

+ dev_dbg(dev, "SEV cmdbuf hi:\t%x\n", pdata->sev_cmd_buf_hi_reg);

s/SEV cmdbuf hi/SEV cmdbuf hi register/

+ dev_dbg(dev, "SEV mbox:\t%x\n", pdata->mbox_irq_id);

s/SEV mbox/SEV cmdresp IRQ/


+ dev_dbg(dev, "ACPI cmdresp:\t%x\n", pdata->acpi_cmd_resp_reg);

Duplicate entry

+#endif
+ }
+ return drvdata;
+}
+
static int sp_get_irqs(struct sp_device *sp)
{
struct sp_platform *sp_platform = sp->dev_specific;
@@ -137,6 +190,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 && pdev->id_entry)

Move this pdev->id_entry check into sp_get_plat_version(), returning NULL if not set.

Thanks,
Tom

+ sp->dev_vdata = sp_get_plat_version(pdev);
if (!sp->dev_vdata) {
ret = -ENODEV;
dev_err(dev, "missing driver data\n");