Re: [PATCH v2 3/9] crypto: ccp: Move some PSP mailbox bit definitions into common header

From: Rijo Thomas
Date: Fri Mar 03 2023 - 05:36:46 EST



On 3/3/2023 1:12 AM, Mario Limonciello wrote:
> Some of the bits and fields used for mailboxes communicating with the
> PSP are common across all mailbox implementations (SEV, TEE, etc).
>
> Move these bits into the common `linux/psp.h` so they don't need to
> be re-defined for each implementation.
>

TEE related changes look fine.

Acked-by: Rijo Thomas <Rijo-john.Thomas@xxxxxxx>

Thanks,
Rijo

> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> v1->v2:
> * Update comment to indicate it's PSP response not PSP ready
> ---
> drivers/crypto/ccp/psp-dev.h | 3 ---
> drivers/crypto/ccp/sev-dev.c | 15 +++++++--------
> drivers/crypto/ccp/sev-dev.h | 2 +-
> drivers/crypto/ccp/tee-dev.c | 15 ++++++++-------
> drivers/i2c/busses/i2c-designware-amdpsp.c | 16 +++++-----------
> include/linux/psp.h | 12 ++++++++++++
> 6 files changed, 33 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/crypto/ccp/psp-dev.h b/drivers/crypto/ccp/psp-dev.h
> index 06e1f317216d..55f54bb2b3fb 100644
> --- a/drivers/crypto/ccp/psp-dev.h
> +++ b/drivers/crypto/ccp/psp-dev.h
> @@ -17,9 +17,6 @@
>
> #include "sp-dev.h"
>
> -#define PSP_CMDRESP_RESP BIT(31)
> -#define PSP_CMDRESP_ERR_MASK 0xffff
> -
> #define MAX_PSP_NAME_LEN 16
>
> extern struct psp_device *psp_master;
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 28945ca7c856..6440d35dfa4e 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -7,6 +7,7 @@
> * Author: Brijesh Singh <brijesh.singh@xxxxxxx>
> */
>
> +#include <linux/bitfield.h>
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/kthread.h>
> @@ -103,7 +104,7 @@ static void sev_irq_handler(int irq, void *data, unsigned int status)
>
> /* Check if it is SEV command completion: */
> reg = ioread32(sev->io_regs + sev->vdata->cmdresp_reg);
> - if (reg & PSP_CMDRESP_RESP) {
> + if (FIELD_GET(PSP_CMDRESP_RESP, reg)) {
> sev->int_rcvd = 1;
> wake_up(&sev->int_queue);
> }
> @@ -347,9 +348,7 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)
>
> sev->int_rcvd = 0;
>
> - reg = cmd;
> - reg <<= SEV_CMDRESP_CMD_SHIFT;
> - reg |= SEV_CMDRESP_IOC;
> + reg = FIELD_PREP(SEV_CMDRESP_CMD, cmd) | SEV_CMDRESP_IOC;
> iowrite32(reg, sev->io_regs + sev->vdata->cmdresp_reg);
>
> /* wait for command completion */
> @@ -367,11 +366,11 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret)
> psp_timeout = psp_cmd_timeout;
>
> if (psp_ret)
> - *psp_ret = reg & PSP_CMDRESP_ERR_MASK;
> + *psp_ret = FIELD_GET(PSP_CMDRESP_STS, reg);
>
> - if (reg & PSP_CMDRESP_ERR_MASK) {
> - dev_dbg(sev->dev, "sev command %#x failed (%#010x)\n",
> - cmd, reg & PSP_CMDRESP_ERR_MASK);
> + if (FIELD_GET(PSP_CMDRESP_STS, reg)) {
> + dev_dbg(sev->dev, "sev command %#x failed (%#010lx)\n",
> + cmd, FIELD_GET(PSP_CMDRESP_STS, reg));
> ret = -EIO;
> } else {
> ret = sev_write_init_ex_file_if_required(cmd);
> diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
> index 666c21eb81ab..778c95155e74 100644
> --- a/drivers/crypto/ccp/sev-dev.h
> +++ b/drivers/crypto/ccp/sev-dev.h
> @@ -25,8 +25,8 @@
> #include <linux/miscdevice.h>
> #include <linux/capability.h>
>
> +#define SEV_CMDRESP_CMD GENMASK(26, 16)
> #define SEV_CMD_COMPLETE BIT(1)
> -#define SEV_CMDRESP_CMD_SHIFT 16
> #define SEV_CMDRESP_IOC BIT(0)
>
> struct sev_misc_dev {
> diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
> index f24fc953718a..5560bf8329a1 100644
> --- a/drivers/crypto/ccp/tee-dev.c
> +++ b/drivers/crypto/ccp/tee-dev.c
> @@ -8,6 +8,7 @@
> * Copyright (C) 2019,2021 Advanced Micro Devices, Inc.
> */
>
> +#include <linux/bitfield.h>
> #include <linux/types.h>
> #include <linux/mutex.h>
> #include <linux/delay.h>
> @@ -69,7 +70,7 @@ static int tee_wait_cmd_poll(struct psp_tee_device *tee, unsigned int timeout,
>
> while (--nloop) {
> *reg = ioread32(tee->io_regs + tee->vdata->cmdresp_reg);
> - if (*reg & PSP_CMDRESP_RESP)
> + if (FIELD_GET(PSP_CMDRESP_RESP, *reg))
> return 0;
>
> usleep_range(10000, 10100);
> @@ -149,9 +150,9 @@ static int tee_init_ring(struct psp_tee_device *tee)
> goto free_buf;
> }
>
> - if (reg & PSP_CMDRESP_ERR_MASK) {
> - dev_err(tee->dev, "tee: ring init command failed (%#010x)\n",
> - reg & PSP_CMDRESP_ERR_MASK);
> + if (FIELD_GET(PSP_CMDRESP_STS, reg)) {
> + dev_err(tee->dev, "tee: ring init command failed (%#010lx)\n",
> + FIELD_GET(PSP_CMDRESP_STS, reg));
> tee_free_ring(tee);
> ret = -EIO;
> }
> @@ -179,9 +180,9 @@ static void tee_destroy_ring(struct psp_tee_device *tee)
> ret = tee_wait_cmd_poll(tee, TEE_DEFAULT_TIMEOUT, &reg);
> if (ret) {
> dev_err(tee->dev, "tee: ring destroy command timed out\n");
> - } else if (reg & PSP_CMDRESP_ERR_MASK) {
> - dev_err(tee->dev, "tee: ring destroy command failed (%#010x)\n",
> - reg & PSP_CMDRESP_ERR_MASK);
> + } else if (FIELD_GET(PSP_CMDRESP_STS, reg)) {
> + dev_err(tee->dev, "tee: ring destroy command failed (%#010lx)\n",
> + FIELD_GET(PSP_CMDRESP_STS, reg));
> }
>
> free_ring:
> diff --git a/drivers/i2c/busses/i2c-designware-amdpsp.c b/drivers/i2c/busses/i2c-designware-amdpsp.c
> index 80f28a1bbbef..652e6b64bd5f 100644
> --- a/drivers/i2c/busses/i2c-designware-amdpsp.c
> +++ b/drivers/i2c/busses/i2c-designware-amdpsp.c
> @@ -25,12 +25,6 @@
> #define PSP_I2C_REQ_STS_BUS_BUSY 0x1
> #define PSP_I2C_REQ_STS_INV_PARAM 0x3
>
> -#define PSP_MBOX_FIELDS_STS GENMASK(15, 0)
> -#define PSP_MBOX_FIELDS_CMD GENMASK(23, 16)
> -#define PSP_MBOX_FIELDS_RESERVED GENMASK(29, 24)
> -#define PSP_MBOX_FIELDS_RECOVERY BIT(30)
> -#define PSP_MBOX_FIELDS_READY BIT(31)
> -
> struct psp_req_buffer_hdr {
> u32 total_size;
> u32 status;
> @@ -99,15 +93,15 @@ static int psp_check_mbox_recovery(struct psp_mbox __iomem *mbox)
>
> tmp = readl(&mbox->cmd_fields);
>
> - return FIELD_GET(PSP_MBOX_FIELDS_RECOVERY, tmp);
> + return FIELD_GET(PSP_CMDRESP_RECOVERY, tmp);
> }
>
> static int psp_wait_cmd(struct psp_mbox __iomem *mbox)
> {
> u32 tmp, expected;
>
> - /* Expect mbox_cmd to be cleared and ready bit to be set by PSP */
> - expected = FIELD_PREP(PSP_MBOX_FIELDS_READY, 1);
> + /* Expect mbox_cmd to be cleared and the response bit to be set by PSP */
> + expected = FIELD_PREP(PSP_CMDRESP_RESP, 1);
>
> /*
> * Check for readiness of PSP mailbox in a tight loop in order to
> @@ -124,7 +118,7 @@ static u32 psp_check_mbox_sts(struct psp_mbox __iomem *mbox)
>
> cmd_reg = readl(&mbox->cmd_fields);
>
> - return FIELD_GET(PSP_MBOX_FIELDS_STS, cmd_reg);
> + return FIELD_GET(PSP_CMDRESP_STS, cmd_reg);
> }
>
> static int psp_send_cmd(struct psp_i2c_req *req)
> @@ -148,7 +142,7 @@ static int psp_send_cmd(struct psp_i2c_req *req)
> writeq(req_addr, &mbox->i2c_req_addr);
>
> /* Write command register to trigger processing */
> - cmd_reg = FIELD_PREP(PSP_MBOX_FIELDS_CMD, PSP_I2C_REQ_BUS_CMD);
> + cmd_reg = FIELD_PREP(PSP_CMDRESP_CMD, PSP_I2C_REQ_BUS_CMD);
> writel(cmd_reg, &mbox->cmd_fields);
>
> if (psp_wait_cmd(mbox))
> diff --git a/include/linux/psp.h b/include/linux/psp.h
> index 202162487ec3..d3424790a70e 100644
> --- a/include/linux/psp.h
> +++ b/include/linux/psp.h
> @@ -11,4 +11,16 @@
> #define __psp_pa(x) __pa(x)
> #endif
>
> +/*
> + * Fields and bits used by most PSP mailboxes
> + *
> + * Note: Some mailboxes (such as SEV) have extra bits or different meanings
> + * and should include an appropriate local definition in their source file.
> + */
> +#define PSP_CMDRESP_STS GENMASK(15, 0)
> +#define PSP_CMDRESP_CMD GENMASK(23, 16)
> +#define PSP_CMDRESP_RESERVED GENMASK(29, 24)
> +#define PSP_CMDRESP_RECOVERY BIT(30)
> +#define PSP_CMDRESP_RESP BIT(31)
> +
> #endif /* __PSP_H */