linux-kernel - Re: [PATCH 17/20] misc: genwqe: card_base: Do not pass unused argument 'fatal

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <016718c7218cf811897cc32cf9476222@linux.vnet.ibm.com>
Date:   Tue, 30 Jun 2020 09:35:23 +0200
From:   haver <haver@...ux.vnet.ibm.com>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     arnd@...db.de, gregkh@...uxfoundation.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Michael Jung <mijung@....net>,
        Michael Ruettger <michael@...ra.de>,
        Frank Haverkamp <haver@...ux.ibm.com>,
        Joerg-Stephan Vogt <jsvogt@...ibm.com>
Subject: Re: [PATCH 17/20] misc: genwqe: card_base: Do not      pass unused
 argument 'fatal_err'

On 2020-06-29 16:04, Lee Jones wrote:
> 'fatal_err' is taken as an argument to a static function which is only
> invoked once.  During this invocation 'fatal_err' is not set.  There
> doesn't appear to be a good reason to keep it around.
> 
> Also fixes the following W=1 kernel build warning:
> 
>  drivers/misc/genwqe/card_base.c:588: warning: Function parameter or
> member 'fatal_err' not described in 'genwqe_recover_card'
> 
> Cc: Michael Jung <mijung@....net>
> Cc: Michael Ruettger <michael@...ra.de>
> Cc: Frank Haverkamp <haver@...ux.ibm.com>
> Cc: Joerg-Stephan Vogt <jsvogt@...ibm.com>
> Signed-off-by: Lee Jones <lee.jones@...aro.org>
> ---
>  drivers/misc/genwqe/card_base.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/misc/genwqe/card_base.c 
> b/drivers/misc/genwqe/card_base.c
> index bceebf49de2d5..809a6f46f6de3 100644
> --- a/drivers/misc/genwqe/card_base.c
> +++ b/drivers/misc/genwqe/card_base.c
> @@ -569,30 +569,18 @@ static int genwqe_stop(struct genwqe_dev *cd)
> 
>  /**
>   * genwqe_recover_card() - Try to recover the card if it is possible
> - *
> - * If fatal_err is set no register access is possible anymore. It is
> - * likely that genwqe_start fails in that situation. Proper error
> - * handling is required in this case.
> + * @cd: GenWQE device information
>   *
>   * genwqe_bus_reset() will cause the pci code to call genwqe_remove()
>   * and later genwqe_probe() for all virtual functions.
>   */
> -static int genwqe_recover_card(struct genwqe_dev *cd, int fatal_err)
> +static int genwqe_recover_card(struct genwqe_dev *cd)
>  {
>  	int rc;
>  	struct pci_dev *pci_dev = cd->pci_dev;
> 
>  	genwqe_stop(cd);
> 
> -	/*
> -	 * Make sure chip is not reloaded to maintain FFDC. Write SLU
> -	 * Reset Register, CPLDReset field to 0.
> -	 */
> -	if (!fatal_err) {
> -		cd->softreset = 0x70ull;
> -		__genwqe_writeq(cd, IO_SLC_CFGREG_SOFTRESET, cd->softreset);
> -	}
> -
>  	rc = genwqe_bus_reset(cd);
>  	if (rc != 0) {
>  		dev_err(&pci_dev->dev,
> @@ -958,7 +946,7 @@ static int genwqe_health_thread(void *data)
> 
>  			cd->card_state = GENWQE_CARD_FATAL_ERROR;
> 
> -			rc = genwqe_recover_card(cd, 0);
> +			rc = genwqe_recover_card(cd);
>  			if (rc < 0) {
>  				/* FIXME Card is unusable and needs unbind! */
>  				goto fatal_error;

I think this one I want to keep. Since fatal_err is 0, !fatal_error is 1 
and the register write is actually executed.
I also want to keep the parameter even though it is only used with 0. 
The register bit causes a less drastic recovery, but if we would 
discover that that is not working ok, we rather discard the debug data 
we get in this case instead of letting the recovery fail. I think it 
does not hurt to keep it. Maybe you can add a comment?

Thanks

Frank