diff mbox series

[v2] bus: mhi: host: Read PK HASH dynamically

Message ID 20240105174253.863388-1-quic_jhugo@quicinc.com (mailing list archive)
State Not Applicable
Headers show
Series [v2] bus: mhi: host: Read PK HASH dynamically | expand

Commit Message

Jeffrey Hugo Jan. 5, 2024, 5:42 p.m. UTC
The OEM PK HASH registers in the BHI region are read once during firmware
load (boot), cached, and displayed on demand via sysfs. This has a few
problems - if firmware load is skipped, the registers will not be read and
if the register values change over the life of the device the local cache
will be out of sync.

Qualcomm Cloud AI 100 can expose both these problems. It is possible for
mhi_async_power_up() to be invoked while the device is in AMSS EE, which
would bypass firmware loading. Also, Qualcomm Cloud AI 100 has 5 PK HASH
slots which can be dynamically provisioned while the device is active,
which would result in the values changing and users may want to know what
keys are active.

Address these concerns by reading the PK HASH registers on-demand during
the sysfs read. This will result in showing the most current information.

Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Reviewed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
---

v2:
-Return ret instead of cnt on error in show()

 drivers/bus/mhi/host/boot.c | 11 +----------
 drivers/bus/mhi/host/init.c | 16 ++++++++++++----
 include/linux/mhi.h         |  2 --
 3 files changed, 13 insertions(+), 16 deletions(-)

Comments

Manivannan Sadhasivam Jan. 6, 2024, 2:44 p.m. UTC | #1
On Fri, Jan 05, 2024 at 10:42:53AM -0700, Jeffrey Hugo wrote:
> The OEM PK HASH registers in the BHI region are read once during firmware
> load (boot), cached, and displayed on demand via sysfs. This has a few
> problems - if firmware load is skipped, the registers will not be read and
> if the register values change over the life of the device the local cache
> will be out of sync.
> 
> Qualcomm Cloud AI 100 can expose both these problems. It is possible for
> mhi_async_power_up() to be invoked while the device is in AMSS EE, which
> would bypass firmware loading. Also, Qualcomm Cloud AI 100 has 5 PK HASH
> slots which can be dynamically provisioned while the device is active,
> which would result in the values changing and users may want to know what
> keys are active.
> 
> Address these concerns by reading the PK HASH registers on-demand during
> the sysfs read. This will result in showing the most current information.
> 
> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> Reviewed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
> ---
> 
> v2:
> -Return ret instead of cnt on error in show()
> 
>  drivers/bus/mhi/host/boot.c | 11 +----------
>  drivers/bus/mhi/host/init.c | 16 ++++++++++++----
>  include/linux/mhi.h         |  2 --
>  3 files changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
> index edc0ec5a0933..dedd29ca8db3 100644
> --- a/drivers/bus/mhi/host/boot.c
> +++ b/drivers/bus/mhi/host/boot.c
> @@ -395,7 +395,7 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>  	void *buf;
>  	dma_addr_t dma_addr;
>  	size_t size, fw_sz;
> -	int i, ret;
> +	int ret;
>  
>  	if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) {
>  		dev_err(dev, "Device MHI is not in valid state\n");
> @@ -408,15 +408,6 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>  	if (ret)
>  		dev_err(dev, "Could not capture serial number via BHI\n");
>  
> -	for (i = 0; i < ARRAY_SIZE(mhi_cntrl->oem_pk_hash); i++) {
> -		ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_OEMPKHASH(i),
> -				   &mhi_cntrl->oem_pk_hash[i]);
> -		if (ret) {
> -			dev_err(dev, "Could not capture OEM PK HASH via BHI\n");
> -			break;
> -		}
> -	}
> -
>  	/* wait for ready on pass through or any other execution environment */
>  	if (!MHI_FW_LOAD_CAPABLE(mhi_cntrl->ee))
>  		goto fw_load_ready_state;
> diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
> index f78aefd2d7a3..15c1740a2c88 100644
> --- a/drivers/bus/mhi/host/init.c
> +++ b/drivers/bus/mhi/host/init.c
> @@ -97,11 +97,19 @@ static ssize_t oem_pk_hash_show(struct device *dev,
>  {
>  	struct mhi_device *mhi_dev = to_mhi_device(dev);
>  	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> -	int i, cnt = 0;
> +	u32 hash_segment[MHI_MAX_OEM_PK_HASH_SEGMENTS];
> +	int i, cnt = 0, ret;
>  
> -	for (i = 0; i < ARRAY_SIZE(mhi_cntrl->oem_pk_hash); i++)
> -		cnt += sysfs_emit_at(buf, cnt, "OEMPKHASH[%d]: 0x%x\n",
> -				i, mhi_cntrl->oem_pk_hash[i]);
> +	for (i = 0; i < MHI_MAX_OEM_PK_HASH_SEGMENTS; i++) {
> +		ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_OEMPKHASH(i), &hash_segment[i]);
> +		if (ret) {
> +			dev_err(dev, "Could not capture OEM PK HASH\n");
> +			return ret;
> +		}
> +	}
> +
> +	for (i = 0; i < MHI_MAX_OEM_PK_HASH_SEGMENTS; i++)
> +		cnt += sysfs_emit_at(buf, cnt, "OEMPKHASH[%d]: 0x%x\n", i, hash_segment[i]);
>  
>  	return cnt;
>  }
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index 039943ec4d4e..e46c68218fe4 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -323,7 +323,6 @@ struct mhi_controller_config {
>   * @major_version: MHI controller major revision number
>   * @minor_version: MHI controller minor revision number
>   * @serial_number: MHI controller serial number obtained from BHI
> - * @oem_pk_hash: MHI controller OEM PK Hash obtained from BHI
>   * @mhi_event: MHI event ring configurations table
>   * @mhi_cmd: MHI command ring configurations table
>   * @mhi_ctxt: MHI device context, shared memory between host and device
> @@ -410,7 +409,6 @@ struct mhi_controller {
>  	u32 major_version;
>  	u32 minor_version;
>  	u32 serial_number;
> -	u32 oem_pk_hash[MHI_MAX_OEM_PK_HASH_SEGMENTS];
>  
>  	struct mhi_event *mhi_event;
>  	struct mhi_cmd *mhi_cmd;
> -- 
> 2.34.1
>
Manivannan Sadhasivam Jan. 30, 2024, 8:17 a.m. UTC | #2
On Fri, Jan 05, 2024 at 10:42:53AM -0700, Jeffrey Hugo wrote:
> The OEM PK HASH registers in the BHI region are read once during firmware
> load (boot), cached, and displayed on demand via sysfs. This has a few
> problems - if firmware load is skipped, the registers will not be read and
> if the register values change over the life of the device the local cache
> will be out of sync.
> 
> Qualcomm Cloud AI 100 can expose both these problems. It is possible for
> mhi_async_power_up() to be invoked while the device is in AMSS EE, which
> would bypass firmware loading. Also, Qualcomm Cloud AI 100 has 5 PK HASH
> slots which can be dynamically provisioned while the device is active,
> which would result in the values changing and users may want to know what
> keys are active.
> 
> Address these concerns by reading the PK HASH registers on-demand during
> the sysfs read. This will result in showing the most current information.
> 
> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> Reviewed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
> ---
> 
> v2:
> -Return ret instead of cnt on error in show()
> 
>  drivers/bus/mhi/host/boot.c | 11 +----------
>  drivers/bus/mhi/host/init.c | 16 ++++++++++++----
>  include/linux/mhi.h         |  2 --
>  3 files changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
> index edc0ec5a0933..dedd29ca8db3 100644
> --- a/drivers/bus/mhi/host/boot.c
> +++ b/drivers/bus/mhi/host/boot.c
> @@ -395,7 +395,7 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>  	void *buf;
>  	dma_addr_t dma_addr;
>  	size_t size, fw_sz;
> -	int i, ret;
> +	int ret;
>  
>  	if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) {
>  		dev_err(dev, "Device MHI is not in valid state\n");
> @@ -408,15 +408,6 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>  	if (ret)
>  		dev_err(dev, "Could not capture serial number via BHI\n");
>  
> -	for (i = 0; i < ARRAY_SIZE(mhi_cntrl->oem_pk_hash); i++) {
> -		ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_OEMPKHASH(i),
> -				   &mhi_cntrl->oem_pk_hash[i]);
> -		if (ret) {
> -			dev_err(dev, "Could not capture OEM PK HASH via BHI\n");
> -			break;
> -		}
> -	}
> -
>  	/* wait for ready on pass through or any other execution environment */
>  	if (!MHI_FW_LOAD_CAPABLE(mhi_cntrl->ee))
>  		goto fw_load_ready_state;
> diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
> index f78aefd2d7a3..15c1740a2c88 100644
> --- a/drivers/bus/mhi/host/init.c
> +++ b/drivers/bus/mhi/host/init.c
> @@ -97,11 +97,19 @@ static ssize_t oem_pk_hash_show(struct device *dev,
>  {
>  	struct mhi_device *mhi_dev = to_mhi_device(dev);
>  	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> -	int i, cnt = 0;
> +	u32 hash_segment[MHI_MAX_OEM_PK_HASH_SEGMENTS];
> +	int i, cnt = 0, ret;
>  
> -	for (i = 0; i < ARRAY_SIZE(mhi_cntrl->oem_pk_hash); i++)
> -		cnt += sysfs_emit_at(buf, cnt, "OEMPKHASH[%d]: 0x%x\n",
> -				i, mhi_cntrl->oem_pk_hash[i]);
> +	for (i = 0; i < MHI_MAX_OEM_PK_HASH_SEGMENTS; i++) {
> +		ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_OEMPKHASH(i), &hash_segment[i]);
> +		if (ret) {
> +			dev_err(dev, "Could not capture OEM PK HASH\n");
> +			return ret;
> +		}
> +	}
> +
> +	for (i = 0; i < MHI_MAX_OEM_PK_HASH_SEGMENTS; i++)
> +		cnt += sysfs_emit_at(buf, cnt, "OEMPKHASH[%d]: 0x%x\n", i, hash_segment[i]);
>  
>  	return cnt;
>  }
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index 039943ec4d4e..e46c68218fe4 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -323,7 +323,6 @@ struct mhi_controller_config {
>   * @major_version: MHI controller major revision number
>   * @minor_version: MHI controller minor revision number
>   * @serial_number: MHI controller serial number obtained from BHI
> - * @oem_pk_hash: MHI controller OEM PK Hash obtained from BHI
>   * @mhi_event: MHI event ring configurations table
>   * @mhi_cmd: MHI command ring configurations table
>   * @mhi_ctxt: MHI device context, shared memory between host and device
> @@ -410,7 +409,6 @@ struct mhi_controller {
>  	u32 major_version;
>  	u32 minor_version;
>  	u32 serial_number;
> -	u32 oem_pk_hash[MHI_MAX_OEM_PK_HASH_SEGMENTS];
>  
>  	struct mhi_event *mhi_event;
>  	struct mhi_cmd *mhi_cmd;
> -- 
> 2.34.1
> 
>
diff mbox series

Patch

diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
index edc0ec5a0933..dedd29ca8db3 100644
--- a/drivers/bus/mhi/host/boot.c
+++ b/drivers/bus/mhi/host/boot.c
@@ -395,7 +395,7 @@  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	void *buf;
 	dma_addr_t dma_addr;
 	size_t size, fw_sz;
-	int i, ret;
+	int ret;
 
 	if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) {
 		dev_err(dev, "Device MHI is not in valid state\n");
@@ -408,15 +408,6 @@  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	if (ret)
 		dev_err(dev, "Could not capture serial number via BHI\n");
 
-	for (i = 0; i < ARRAY_SIZE(mhi_cntrl->oem_pk_hash); i++) {
-		ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_OEMPKHASH(i),
-				   &mhi_cntrl->oem_pk_hash[i]);
-		if (ret) {
-			dev_err(dev, "Could not capture OEM PK HASH via BHI\n");
-			break;
-		}
-	}
-
 	/* wait for ready on pass through or any other execution environment */
 	if (!MHI_FW_LOAD_CAPABLE(mhi_cntrl->ee))
 		goto fw_load_ready_state;
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..15c1740a2c88 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -97,11 +97,19 @@  static ssize_t oem_pk_hash_show(struct device *dev,
 {
 	struct mhi_device *mhi_dev = to_mhi_device(dev);
 	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
-	int i, cnt = 0;
+	u32 hash_segment[MHI_MAX_OEM_PK_HASH_SEGMENTS];
+	int i, cnt = 0, ret;
 
-	for (i = 0; i < ARRAY_SIZE(mhi_cntrl->oem_pk_hash); i++)
-		cnt += sysfs_emit_at(buf, cnt, "OEMPKHASH[%d]: 0x%x\n",
-				i, mhi_cntrl->oem_pk_hash[i]);
+	for (i = 0; i < MHI_MAX_OEM_PK_HASH_SEGMENTS; i++) {
+		ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_OEMPKHASH(i), &hash_segment[i]);
+		if (ret) {
+			dev_err(dev, "Could not capture OEM PK HASH\n");
+			return ret;
+		}
+	}
+
+	for (i = 0; i < MHI_MAX_OEM_PK_HASH_SEGMENTS; i++)
+		cnt += sysfs_emit_at(buf, cnt, "OEMPKHASH[%d]: 0x%x\n", i, hash_segment[i]);
 
 	return cnt;
 }
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 039943ec4d4e..e46c68218fe4 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -323,7 +323,6 @@  struct mhi_controller_config {
  * @major_version: MHI controller major revision number
  * @minor_version: MHI controller minor revision number
  * @serial_number: MHI controller serial number obtained from BHI
- * @oem_pk_hash: MHI controller OEM PK Hash obtained from BHI
  * @mhi_event: MHI event ring configurations table
  * @mhi_cmd: MHI command ring configurations table
  * @mhi_ctxt: MHI device context, shared memory between host and device
@@ -410,7 +409,6 @@  struct mhi_controller {
 	u32 major_version;
 	u32 minor_version;
 	u32 serial_number;
-	u32 oem_pk_hash[MHI_MAX_OEM_PK_HASH_SEGMENTS];
 
 	struct mhi_event *mhi_event;
 	struct mhi_cmd *mhi_cmd;