Re: [PATCH v5 18/34] cxlflash: Fix AFU version access/storage and add check

[Daniel Axtens <dja@axtens.net>]

"Matthew R. Ochs" <mrochs@linux.vnet.ibm.com> writes:

> The AFU version is stored as a non-terminated string of bytes within
> a 64-bit little-endian register. Presently the value is read directly
> (no MMIO accessor) and is stored in a buffer that is not big enough
> to contain a NULL terminator. Additionally the version obtained is not
> evaluated against a known value to prevent usage with unsupported AFUs.
> All of these deficiencies can lead to a variety of problems.
>
> +	if ((afu->interface_version + 1) == 0) {
> +		pr_err("Back level AFU, please upgrade. AFU version %s "
> +		       "interface version 0x%llx\n", afu->version,
> +		       afu->interface_version);
> +		rc = -EINVAL;
> +		goto err1;

I'm confused by this if statement. If afu->interface_version + 1 == 0,
and interface_version is a 64bit unsigned int, that would mean that
afu->interface_version was 0xFFFF FFFF FFFF FFFF.

Are you trying to check against all Fs? Is that value significant in the
hardware?

Regards,
Daniel



> +	} else
> +		pr_debug("%s: afu version %s, interface version 0x%llX\n",
> +			 __func__, afu->version, afu->interface_version);
>  
>  	rc = start_afu(cfg);
>  	if (rc) {
> diff --git a/drivers/scsi/cxlflash/sislite.h b/drivers/scsi/cxlflash/sislite.h
> index 63bf394..8425d1a 100644
> --- a/drivers/scsi/cxlflash/sislite.h
> +++ b/drivers/scsi/cxlflash/sislite.h
> @@ -340,7 +340,7 @@ struct sisl_global_regs {
>  #define SISL_AFUCONF_MBOX_CLR_READ     0x0010ULL
>  	__be64 afu_config;
>  	__be64 rsvd[0xf8];
> -	__be64 afu_version;
> +	__le64 afu_version;
>  	__be64 interface_version;
>  };
>  
> -- 
> 2.1.0
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev