Re: [PATCH] bus: mhi: host: Read PK HASH dynamically

[Jeffrey Hugo <quic_jhugo@xxxxxxxxxxx>]

On 1/3/2024 11:46 PM, Manivannan Sadhasivam wrote:
> On Fri, Dec 08, 2023 at 09:59:38AM -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@xxxxxxxxxxx>
> > Reviewed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@xxxxxxxxxxx>
> > ---
> >   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 65ceac1837f9..e2c2f510b04f 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 cnt;
>
> return ret?

Sure.

> - Mani
>
> > +		}
> > +	}
> > +
> > +	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 d0f9b522f328..474d32cb0520 100644
> > --- a/include/linux/mhi.h
> > +++ b/include/linux/mhi.h
> > @@ -325,7 +325,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
> > @@ -413,7 +412,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