Re: [PATCH v6 3/4] i2c: designware: Use PCI PSP driver for communication

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 22, 2023 at 04:02:25PM -0500, Mario Limonciello wrote:
> Currently the PSP semaphore communication base address is discovered
> by using an MSR that is not architecturally guaranteed for future
> platforms.  Also the mailbox that is utilized for communication with
> the PSP may have other consumers in the kernel, so it's better to
> make all communication go through a single driver.

Fine by me,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> v5->v6:
>  * Drop now unnecessary asm/msr.h header
>  * Fix IS_REACHABLE
>  * Drop tags
>  * Fix status code handling for Cezanne
> v4->v5:
>  * Pick up tags
> v3->v4:
>  * Pick up tags
> v1->v2:
>  * Fix Kconfig to use imply
>  * Use IS_REACHABLE
> ---
>  drivers/i2c/busses/Kconfig                  |   2 +-
>  drivers/i2c/busses/i2c-designware-amdpsp.c  | 175 +++-----------------
>  drivers/i2c/busses/i2c-designware-core.h    |   1 -
>  drivers/i2c/busses/i2c-designware-platdrv.c |   1 -
>  include/linux/psp-platform-access.h         |   1 +
>  5 files changed, 27 insertions(+), 153 deletions(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 25eb4e8fd22f..d53bf716f97d 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -566,9 +566,9 @@ config I2C_DESIGNWARE_PLATFORM
>  
>  config I2C_DESIGNWARE_AMDPSP
>  	bool "AMD PSP I2C semaphore support"
> -	depends on X86_MSR
>  	depends on ACPI
>  	depends on I2C_DESIGNWARE_PLATFORM
> +	imply CRYPTO_DEV_SP_PSP
>  	help
>  	  This driver enables managed host access to the selected I2C bus shared
>  	  between AMD CPU and AMD PSP.
> diff --git a/drivers/i2c/busses/i2c-designware-amdpsp.c b/drivers/i2c/busses/i2c-designware-amdpsp.c
> index 652e6b64bd5f..12870dc44bdb 100644
> --- a/drivers/i2c/busses/i2c-designware-amdpsp.c
> +++ b/drivers/i2c/busses/i2c-designware-amdpsp.c
> @@ -1,35 +1,20 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> -#include <linux/bitfield.h>
> -#include <linux/bits.h>
>  #include <linux/i2c.h>
> -#include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/psp-platform-access.h>
>  #include <linux/psp.h>
> -#include <linux/types.h>
>  #include <linux/workqueue.h>
>  
> -#include <asm/msr.h>
> -
>  #include "i2c-designware-core.h"
>  
> -#define MSR_AMD_PSP_ADDR	0xc00110a2
> -#define PSP_MBOX_OFFSET		0x10570
> -#define PSP_CMD_TIMEOUT_US	(500 * USEC_PER_MSEC)
> -
>  #define PSP_I2C_RESERVATION_TIME_MS 100
>  
> -#define PSP_I2C_REQ_BUS_CMD		0x64
>  #define PSP_I2C_REQ_RETRY_CNT		400
>  #define PSP_I2C_REQ_RETRY_DELAY_US	(25 * USEC_PER_MSEC)
>  #define PSP_I2C_REQ_STS_OK		0x0
>  #define PSP_I2C_REQ_STS_BUS_BUSY	0x1
>  #define PSP_I2C_REQ_STS_INV_PARAM	0x3
>  
> -struct psp_req_buffer_hdr {
> -	u32 total_size;
> -	u32 status;
> -};
> -
>  enum psp_i2c_req_type {
>  	PSP_I2C_REQ_ACQUIRE,
>  	PSP_I2C_REQ_RELEASE,
> @@ -41,119 +26,12 @@ struct psp_i2c_req {
>  	enum psp_i2c_req_type type;
>  };
>  
> -struct psp_mbox {
> -	u32 cmd_fields;
> -	u64 i2c_req_addr;
> -} __packed;
> -
>  static DEFINE_MUTEX(psp_i2c_access_mutex);
>  static unsigned long psp_i2c_sem_acquired;
> -static void __iomem *mbox_iomem;
>  static u32 psp_i2c_access_count;
>  static bool psp_i2c_mbox_fail;
>  static struct device *psp_i2c_dev;
>  
> -/*
> - * Implementation of PSP-x86 i2c-arbitration mailbox introduced for AMD Cezanne
> - * family of SoCs.
> - */
> -
> -static int psp_get_mbox_addr(unsigned long *mbox_addr)
> -{
> -	unsigned long long psp_mmio;
> -
> -	if (rdmsrl_safe(MSR_AMD_PSP_ADDR, &psp_mmio))
> -		return -EIO;
> -
> -	*mbox_addr = (unsigned long)(psp_mmio + PSP_MBOX_OFFSET);
> -
> -	return 0;
> -}
> -
> -static int psp_mbox_probe(void)
> -{
> -	unsigned long mbox_addr;
> -	int ret;
> -
> -	ret = psp_get_mbox_addr(&mbox_addr);
> -	if (ret)
> -		return ret;
> -
> -	mbox_iomem = ioremap(mbox_addr, sizeof(struct psp_mbox));
> -	if (!mbox_iomem)
> -		return -ENOMEM;
> -
> -	return 0;
> -}
> -
> -/* Recovery field should be equal 0 to start sending commands */
> -static int psp_check_mbox_recovery(struct psp_mbox __iomem *mbox)
> -{
> -	u32 tmp;
> -
> -	tmp = readl(&mbox->cmd_fields);
> -
> -	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 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
> -	 * process further as soon as command was consumed.
> -	 */
> -	return readl_poll_timeout(&mbox->cmd_fields, tmp, (tmp == expected),
> -				  0, PSP_CMD_TIMEOUT_US);
> -}
> -
> -/* Status equal to 0 means that PSP succeed processing command */
> -static u32 psp_check_mbox_sts(struct psp_mbox __iomem *mbox)
> -{
> -	u32 cmd_reg;
> -
> -	cmd_reg = readl(&mbox->cmd_fields);
> -
> -	return FIELD_GET(PSP_CMDRESP_STS, cmd_reg);
> -}
> -
> -static int psp_send_cmd(struct psp_i2c_req *req)
> -{
> -	struct psp_mbox __iomem *mbox = mbox_iomem;
> -	phys_addr_t req_addr;
> -	u32 cmd_reg;
> -
> -	if (psp_check_mbox_recovery(mbox))
> -		return -EIO;
> -
> -	if (psp_wait_cmd(mbox))
> -		return -EBUSY;
> -
> -	/*
> -	 * Fill mailbox with address of command-response buffer, which will be
> -	 * used for sending i2c requests as well as reading status returned by
> -	 * PSP. Use physical address of buffer, since PSP will map this region.
> -	 */
> -	req_addr = __psp_pa((void *)req);
> -	writeq(req_addr, &mbox->i2c_req_addr);
> -
> -	/* Write command register to trigger processing */
> -	cmd_reg = FIELD_PREP(PSP_CMDRESP_CMD, PSP_I2C_REQ_BUS_CMD);
> -	writel(cmd_reg, &mbox->cmd_fields);
> -
> -	if (psp_wait_cmd(mbox))
> -		return -ETIMEDOUT;
> -
> -	if (psp_check_mbox_sts(mbox))
> -		return -EIO;
> -
> -	return 0;
> -}
> -
>  /* Helper to verify status returned by PSP */
>  static int check_i2c_req_sts(struct psp_i2c_req *req)
>  {
> @@ -173,22 +51,25 @@ static int check_i2c_req_sts(struct psp_i2c_req *req)
>  	}
>  }
>  
> -static int psp_send_check_i2c_req(struct psp_i2c_req *req)
> +/*
> + * Errors in x86-PSP i2c-arbitration protocol may occur at two levels:
> + * 1. mailbox communication - PSP is not operational or some IO errors with
> + *    basic communication had happened.
> + * 2. i2c-requests - PSP refuses to grant i2c arbitration to x86 for too long.
> + *
> + * In order to distinguish between these in error handling code all mailbox
> + * communication errors on the first level (from CCP symbols) will be passed
> + * up and if -EIO is returned the second level will be checked.
> + */
> +static int psp_send_i2c_req_cezanne(struct psp_i2c_req *req)
>  {
> -	/*
> -	 * Errors in x86-PSP i2c-arbitration protocol may occur at two levels:
> -	 * 1. mailbox communication - PSP is not operational or some IO errors
> -	 * with basic communication had happened;
> -	 * 2. i2c-requests - PSP refuses to grant i2c arbitration to x86 for too
> -	 * long.
> -	 * In order to distinguish between these two in error handling code, all
> -	 * errors on the first level (returned by psp_send_cmd) are shadowed by
> -	 * -EIO.
> -	 */
> -	if (psp_send_cmd(req))
> -		return -EIO;
> +	int ret;
>  
> -	return check_i2c_req_sts(req);
> +	ret = psp_send_platform_access_msg(PSP_I2C_REQ_BUS_CMD, (struct psp_request *)req);
> +	if (ret == -EIO)
> +		return check_i2c_req_sts(req);
> +
> +	return ret;
>  }
>  
>  static int psp_send_i2c_req(enum psp_i2c_req_type i2c_req_type)
> @@ -202,11 +83,11 @@ static int psp_send_i2c_req(enum psp_i2c_req_type i2c_req_type)
>  	if (!req)
>  		return -ENOMEM;
>  
> -	req->hdr.total_size = sizeof(*req);
> +	req->hdr.payload_size = sizeof(*req);
>  	req->type = i2c_req_type;
>  
>  	start = jiffies;
> -	ret = read_poll_timeout(psp_send_check_i2c_req, status,
> +	ret = read_poll_timeout(psp_send_i2c_req_cezanne, status,
>  				(status != -EBUSY),
>  				PSP_I2C_REQ_RETRY_DELAY_US,
>  				PSP_I2C_REQ_RETRY_CNT * PSP_I2C_REQ_RETRY_DELAY_US,
> @@ -381,7 +262,8 @@ static const struct i2c_lock_operations i2c_dw_psp_lock_ops = {
>  
>  int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev)
>  {
> -	int ret;
> +	if (!IS_REACHABLE(CONFIG_CRYPTO_DEV_CCP_DD))
> +		return -ENODEV;
>  
>  	if (!dev)
>  		return -ENODEV;
> @@ -393,11 +275,10 @@ int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev)
>  	if (psp_i2c_dev)
>  		return -EEXIST;
>  
> -	psp_i2c_dev = dev->dev;
> +	if (psp_check_platform_access_status())
> +		return -EPROBE_DEFER;
>  
> -	ret = psp_mbox_probe();
> -	if (ret)
> -		return ret;
> +	psp_i2c_dev = dev->dev;
>  
>  	dev_info(psp_i2c_dev, "I2C bus managed by AMD PSP\n");
>  
> @@ -411,9 +292,3 @@ int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev)
>  
>  	return 0;
>  }
> -
> -/* Unmap area used as a mailbox with PSP */
> -void i2c_dw_amdpsp_remove_lock_support(struct dw_i2c_dev *dev)
> -{
> -	iounmap(mbox_iomem);
> -}
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index 050d8c63ad3c..c5d87aae39c6 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -383,7 +383,6 @@ int i2c_dw_baytrail_probe_lock_support(struct dw_i2c_dev *dev);
>  
>  #if IS_ENABLED(CONFIG_I2C_DESIGNWARE_AMDPSP)
>  int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev);
> -void i2c_dw_amdpsp_remove_lock_support(struct dw_i2c_dev *dev);
>  #endif
>  
>  int i2c_dw_validate_speed(struct dw_i2c_dev *dev);
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 74182db03a88..89ad88c54754 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -214,7 +214,6 @@ static const struct i2c_dw_semaphore_callbacks i2c_dw_semaphore_cb_table[] = {
>  #ifdef CONFIG_I2C_DESIGNWARE_AMDPSP
>  	{
>  		.probe = i2c_dw_amdpsp_probe_lock_support,
> -		.remove = i2c_dw_amdpsp_remove_lock_support,
>  	},
>  #endif
>  	{}
> diff --git a/include/linux/psp-platform-access.h b/include/linux/psp-platform-access.h
> index 1b661341d8f3..75da8f5f7ad8 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_I2C_REQ_BUS_CMD = 0x64,
>  };
>  
>  struct psp_req_buffer_hdr {
> -- 
> 2.34.1
> 

-- 
With Best Regards,
Andy Shevchenko





[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux