linux-kernel - Re: [PATCH 2/2] fsi: scom: Remove retries in indirect scoms

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <82083174-fccb-a9e2-616f-7a706958e2da@linux.ibm.com>
Date:   Mon, 10 Jan 2022 17:13:45 -0600
From:   Eddie James <eajames@...ux.ibm.com>
To:     Joel Stanley <joel@....id.au>, Jeremy Kerr <jk@...abs.org>,
        Alistar Popple <alistair@...ple.id.au>
Cc:     linux-fsi@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Greg KH <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 2/2] fsi: scom: Remove retries in indirect scoms


On 12/6/21 21:38, Joel Stanley wrote:
> In commit f72ddbe1d7b7 ("fsi: scom: Remove retries") the retries were
> removed from get and put scoms. That patch missed the retires in get and
> put indirect scom.
>
> For the same reason, remove them from the scom driver to allow the
> caller to decide to retry.
>
> This removes the following special case which would have caused the
> retry code to return early:
>
>   -       if ((ind_data & XSCOM_DATA_IND_COMPLETE) || (err != SCOM_PIB_BLOCKED))
>   -               return 0;
>
> I believe this case is handled.


Reviewed-by: Eddie James <eajames@...ux.ibm.com>


>
> Fixes: f72ddbe1d7b7 ("fsi: scom: Remove retries")
> Signed-off-by: Joel Stanley <joel@....id.au>
> ---
>   drivers/fsi/fsi-scom.c | 41 +++++++++++++++--------------------------
>   1 file changed, 15 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c
> index 3b427f7e9027..bcb756dc9866 100644
> --- a/drivers/fsi/fsi-scom.c
> +++ b/drivers/fsi/fsi-scom.c
> @@ -145,7 +145,7 @@ static int put_indirect_scom_form0(struct scom_device *scom, uint64_t value,
>   				   uint64_t addr, uint32_t *status)
>   {
>   	uint64_t ind_data, ind_addr;
> -	int rc, retries, err = 0;
> +	int rc, err;
>   
>   	if (value & ~XSCOM_DATA_IND_DATA)
>   		return -EINVAL;
> @@ -156,19 +156,14 @@ static int put_indirect_scom_form0(struct scom_device *scom, uint64_t value,
>   	if (rc || (*status & SCOM_STATUS_ANY_ERR))
>   		return rc;
>   
> -	for (retries = 0; retries < SCOM_MAX_IND_RETRIES; retries++) {
> -		rc = __get_scom(scom, &ind_data, addr, status);
> -		if (rc || (*status & SCOM_STATUS_ANY_ERR))
> -			return rc;
> +	rc = __get_scom(scom, &ind_data, addr, status);
> +	if (rc || (*status & SCOM_STATUS_ANY_ERR))
> +		return rc;
>   
> -		err = (ind_data & XSCOM_DATA_IND_ERR_MASK) >> XSCOM_DATA_IND_ERR_SHIFT;
> -		*status = err << SCOM_STATUS_PIB_RESP_SHIFT;
> -		if ((ind_data & XSCOM_DATA_IND_COMPLETE) || (err != SCOM_PIB_BLOCKED))
> -			return 0;
> +	err = (ind_data & XSCOM_DATA_IND_ERR_MASK) >> XSCOM_DATA_IND_ERR_SHIFT;
> +	*status = err << SCOM_STATUS_PIB_RESP_SHIFT;
>   
> -		msleep(1);
> -	}
> -	return rc;
> +	return 0;
>   }
>   
>   static int put_indirect_scom_form1(struct scom_device *scom, uint64_t value,
> @@ -188,7 +183,7 @@ static int get_indirect_scom_form0(struct scom_device *scom, uint64_t *value,
>   				   uint64_t addr, uint32_t *status)
>   {
>   	uint64_t ind_data, ind_addr;
> -	int rc, retries, err = 0;
> +	int rc, err;
>   
>   	ind_addr = addr & XSCOM_ADDR_DIRECT_PART;
>   	ind_data = (addr & XSCOM_ADDR_INDIRECT_PART) | XSCOM_DATA_IND_READ;
> @@ -196,21 +191,15 @@ static int get_indirect_scom_form0(struct scom_device *scom, uint64_t *value,
>   	if (rc || (*status & SCOM_STATUS_ANY_ERR))
>   		return rc;
>   
> -	for (retries = 0; retries < SCOM_MAX_IND_RETRIES; retries++) {
> -		rc = __get_scom(scom, &ind_data, addr, status);
> -		if (rc || (*status & SCOM_STATUS_ANY_ERR))
> -			return rc;
> -
> -		err = (ind_data & XSCOM_DATA_IND_ERR_MASK) >> XSCOM_DATA_IND_ERR_SHIFT;
> -		*status = err << SCOM_STATUS_PIB_RESP_SHIFT;
> -		*value = ind_data & XSCOM_DATA_IND_DATA;
> +	rc = __get_scom(scom, &ind_data, addr, status);
> +	if (rc || (*status & SCOM_STATUS_ANY_ERR))
> +		return rc;
>   
> -		if ((ind_data & XSCOM_DATA_IND_COMPLETE) || (err != SCOM_PIB_BLOCKED))
> -			return 0;
> +	err = (ind_data & XSCOM_DATA_IND_ERR_MASK) >> XSCOM_DATA_IND_ERR_SHIFT;
> +	*status = err << SCOM_STATUS_PIB_RESP_SHIFT;
> +	*value = ind_data & XSCOM_DATA_IND_DATA;
>   
> -		msleep(1);
> -	}
> -	return rc;
> +	return 0;
>   }
>   
>   static int raw_put_scom(struct scom_device *scom, uint64_t value,