[PATCH 1/4] lpfc: fix calls to dma_set_mask_and_coherent()

[PATCH 1/4] lpfc: fix calls to dma_set_mask_and_coherent()

[Hannes Reinecke]

The change to use dma_set_mask_and_coherent() incorrectly made a second
call with the 32 bit DMA mask value when the call with the 64 bit DMA
mask value succeeded.  This resulted in NVMe/FC connections failing due
to corrupted data buffers, and various other SCSI/FCP I/O errors.

Fixes: f30e1bfd6154 ("scsi: lpfc: use dma_set_mask_and_coherent")
Cc: <stable@vger.kernel.org>
Suggested-by: Don Dutile <ddutile@redhat.com>
Signed-off-by: Ewan D. Milne <emilne@redhat.com>
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/lpfc/lpfc_init.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index bede11e16349..8a4b096ffe47 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -7361,15 +7361,18 @@ lpfc_sli_pci_mem_setup(struct lpfc_hba *phba)
 	unsigned long bar0map_len, bar2map_len;
 	int i, hbq_count;
 	void *ptr;
-	int error = -ENODEV;
+	int error;
 
 	if (!pdev)
 		return error;
 
 	/* Set the device DMA mask size */
-	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) ||
-	    dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
+	error = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+	if (error)
+		error = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (error)
 		return error;
+	error = -ENODEV;
 
 	/* Get the bus address of Bar0 and Bar2 and the number of bytes
 	 * required by each mapping.
-- 
2.16.4

Re: [PATCH 1/4] lpfc: fix calls to dma_set_mask_and_coherent()

[Ewan D. Milne]

See below.  Also, this patch only fixes 1 of the 2 places,
we need to fix lpfc_sli4_pci_mem_setup() also.  With the
same code applied in both places, the NVMe/FC issue is fixed.
I expect the SCSI/FCP tests would pass too, but those take
a couple of days.

-Ewan

On Wed, 2019-02-13 at 12:42 +0100, Hannes Reinecke wrote:
> The change to use dma_set_mask_and_coherent() incorrectly made a second
> call with the 32 bit DMA mask value when the call with the 64 bit DMA
> mask value succeeded.  This resulted in NVMe/FC connections failing due
> to corrupted data buffers, and various other SCSI/FCP I/O errors.
> 
> Fixes: f30e1bfd6154 ("scsi: lpfc: use dma_set_mask_and_coherent")
> Cc: <stable@vger.kernel.org>
> Suggested-by: Don Dutile <ddutile@redhat.com>
> Signed-off-by: Ewan D. Milne <emilne@redhat.com>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index bede11e16349..8a4b096ffe47 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -7361,15 +7361,18 @@ lpfc_sli_pci_mem_setup(struct lpfc_hba *phba)
>  	unsigned long bar0map_len, bar2map_len;
>  	int i, hbq_count;
>  	void *ptr;
> -	int error = -ENODEV;
> +	int error;
>  
>  	if (!pdev)
>  		return error;

Since this patch removes the initialization of "error" above, this return
statement returns an undefined value.  Change it to return -ENODEV?

>  
>  	/* Set the device DMA mask size */
> -	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) ||
> -	    dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
> +	error = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +	if (error)
> +		error = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	if (error)
>  		return error;
> +	error = -ENODEV;
>  
>  	/* Get the bus address of Bar0 and Bar2 and the number of bytes
>  	 * required by each mapping.

[PATCH 2/4] hptiop: fix calls to dma_set_mask_and_coherent()

[Hannes Reinecke]

The change to use dma_set_mask_and_coherent() incorrectly made a second
call with the 32 bit DMA mask value when the call with the 64 bit DMA
mask value succeeded.  This resulted in NVMe/FC connections failing due
to corrupted data buffers, and various other SCSI/FCP I/O errors.

Fixes: 453cd3700ca3 ("scsi: hptiop: use dma_set_mask")
Cc: <stable@vger.kernel.org>
Suggested-by: Ewan D. Milne <emilne@redhat.com>
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/hptiop.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
index 3eedfd4f8f57..251c084a6ff0 100644
--- a/drivers/scsi/hptiop.c
+++ b/drivers/scsi/hptiop.c
@@ -1292,6 +1292,7 @@ static int hptiop_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
 	dma_addr_t start_phy;
 	void *start_virt;
 	u32 offset, i, req_size;
+	int rc;
 
 	dprintk("hptiop_probe(%p)\n", pcidev);
 
@@ -1308,9 +1309,12 @@ static int hptiop_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
 
 	/* Enable 64bit DMA if possible */
 	iop_ops = (struct hptiop_adapter_ops *)id->driver_data;
-	if (dma_set_mask(&pcidev->dev,
-			 DMA_BIT_MASK(iop_ops->hw_dma_bit_mask)) ||
-	    dma_set_mask(&pcidev->dev, DMA_BIT_MASK(32))) {
+	rc = dma_set_mask(&pcidev->dev,
+			  DMA_BIT_MASK(iop_ops->hw_dma_bit_mask));
+	if (rc)
+		rc = dma_set_mask(&pcidev->dev, DMA_BIT_MASK(32));
+
+	if (rc) {
 		printk(KERN_ERR "hptiop: fail to set dma_mask\n");
 		goto disable_pci_device;
 	}
-- 
2.16.4

Re: [PATCH 2/4] hptiop: fix calls to dma_set_mask_and_coherent()

[Ewan D. Milne]

The patch subject line is incorrect, the driver was changed
to call dma_set_mask(), not dma_set_mask_and_coherent().
Also see comments below re: the patch description.  The
code itself looks fine.

-Ewan

On Wed, 2019-02-13 at 12:42 +0100, Hannes Reinecke wrote:
> The change to use dma_set_mask_and_coherent() incorrectly made a second
> call with the 32 bit DMA mask value when the call with the 64 bit DMA
                                                             ======
> mask value succeeded.  This resulted in NVMe/FC connections failing due
                         ------------------------------------------------
> to corrupted data buffers, and various other SCSI/FCP I/O errors.
-------------------------------------------------------------------

The last sentence should be removed from the patch description.  And the
second-to-last sentence should say something like "..the first DMA mask...".

> 
> Fixes: 453cd3700ca3 ("scsi: hptiop: use dma_set_mask")
> Cc: <stable@vger.kernel.org>
> Suggested-by: Ewan D. Milne <emilne@redhat.com>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/hptiop.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
> index 3eedfd4f8f57..251c084a6ff0 100644
> --- a/drivers/scsi/hptiop.c
> +++ b/drivers/scsi/hptiop.c
> @@ -1292,6 +1292,7 @@ static int hptiop_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
>  	dma_addr_t start_phy;
>  	void *start_virt;
>  	u32 offset, i, req_size;
> +	int rc;
>  
>  	dprintk("hptiop_probe(%p)\n", pcidev);
>  
> @@ -1308,9 +1309,12 @@ static int hptiop_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
>  
>  	/* Enable 64bit DMA if possible */
>  	iop_ops = (struct hptiop_adapter_ops *)id->driver_data;
> -	if (dma_set_mask(&pcidev->dev,
> -			 DMA_BIT_MASK(iop_ops->hw_dma_bit_mask)) ||
> -	    dma_set_mask(&pcidev->dev, DMA_BIT_MASK(32))) {
> +	rc = dma_set_mask(&pcidev->dev,
> +			  DMA_BIT_MASK(iop_ops->hw_dma_bit_mask));
> +	if (rc)
> +		rc = dma_set_mask(&pcidev->dev, DMA_BIT_MASK(32));
> +
> +	if (rc) {
>  		printk(KERN_ERR "hptiop: fail to set dma_mask\n");
>  		goto disable_pci_device;
>  	}

[PATCH 3/4] bfa: fix calls to dma_set_mask_and_coherent()

[Hannes Reinecke]

The change to use dma_set_mask_and_coherent() incorrectly made a second
call with the 32 bit DMA mask value when the call with the 64 bit DMA
mask value succeeded.  This resulted in FC connections failing due
to corrupted data buffers, and various other SCSI/FCP I/O errors.

Fixes: a69b080025ea ("scsi: bfa: use dma_set_mask_and_coherent")
Cc: <stable@vger.kernel.org>
Suggested-by: Ewan D. Milne <emilne@redhat.com>
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/bfa/bfad.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index 42a0caf6740d..2ffbe36f5860 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -727,7 +727,7 @@ bfad_init_timer(struct bfad_s *bfad)
 int
 bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
 {
-	int		rc = -ENODEV;
+	int rc;
 
 	if (pci_enable_device(pdev)) {
 		printk(KERN_ERR "pci_enable_device fail %p\n", pdev);
@@ -739,11 +739,15 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
 
 	pci_set_master(pdev);
 
-	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) ||
-	    dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32))) {
+	rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+	if (rc)
+		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+
+	if (rc) {
 		printk(KERN_ERR "dma_set_mask_and_coherent fail %p\n", pdev);
 		goto out_release_region;
 	}
+	rc = -ENODEV;
 
 	/* Enable PCIE Advanced Error Recovery (AER) if kernel supports */
 	pci_enable_pcie_error_reporting(pdev);
-- 
2.16.4

Re: [PATCH 3/4] bfa: fix calls to dma_set_mask_and_coherent()

[Ewan D. Milne]

See below.  Also see comment re: the patch description.

-Ewan

On Wed, 2019-02-13 at 12:42 +0100, Hannes Reinecke wrote:
> The change to use dma_set_mask_and_coherent() incorrectly made a second
> call with the 32 bit DMA mask value when the call with the 64 bit DMA
> mask value succeeded.  This resulted in FC connections failing due
                         -------------------------------------------
> to corrupted data buffers, and various other SCSI/FCP I/O errors.
  -----------------------------------------------------------------

The last sentence should be removed from the patch description.

> 
> Fixes: a69b080025ea ("scsi: bfa: use dma_set_mask_and_coherent")
> Cc: <stable@vger.kernel.org>
> Suggested-by: Ewan D. Milne <emilne@redhat.com>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/bfa/bfad.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
> index 42a0caf6740d..2ffbe36f5860 100644
> --- a/drivers/scsi/bfa/bfad.c
> +++ b/drivers/scsi/bfa/bfad.c
> @@ -727,7 +727,7 @@ bfad_init_timer(struct bfad_s *bfad)
>  int
>  bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>  {
> -	int		rc = -ENODEV;
> +	int rc;

There are error paths from the calls to pci_enable_device() and
pci_request_regions() that will return an undefined value if this
initializer is removed.  Leave it in place?

>  
>  	if (pci_enable_device(pdev)) {
>  		printk(KERN_ERR "pci_enable_device fail %p\n", pdev);
> @@ -739,11 +739,15 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>  
>  	pci_set_master(pdev);
>  
> -	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)) ||
> -	    dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32))) {
> +	rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +	if (rc)
> +		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +
> +	if (rc) {
>  		printk(KERN_ERR "dma_set_mask_and_coherent fail %p\n", pdev);
>  		goto out_release_region;
>  	}
> +	rc = -ENODEV;
>  
>  	/* Enable PCIE Advanced Error Recovery (AER) if kernel supports */
>  	pci_enable_pcie_error_reporting(pdev);

Re: [PATCH 3/4] bfa: fix calls to dma_set_mask_and_coherent()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4140 bytes --]

Hi Hannes,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on v5.0-rc4 next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Hannes-Reinecke/scsi-fixup-dma_set_mask_and_coherent-calls/20190214-044535
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=xtensa 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/scsi/bfa/bfad.c: In function 'bfad_pci_init':
>> drivers/scsi/bfa/bfad.c:730:6: warning: 'rc' may be used uninitialized in this function [-Wmaybe-uninitialized]
     int rc;
         ^~

vim +/rc +730 drivers/scsi/bfa/bfad.c

   726	
   727	int
   728	bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
   729	{
 > 730		int rc;
   731	
   732		if (pci_enable_device(pdev)) {
   733			printk(KERN_ERR "pci_enable_device fail %p\n", pdev);
   734			goto out;
   735		}
   736	
   737		if (pci_request_regions(pdev, BFAD_DRIVER_NAME))
   738			goto out_disable_device;
   739	
   740		pci_set_master(pdev);
   741	
   742		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
   743		if (rc)
   744			rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
   745	
   746		if (rc) {
   747			printk(KERN_ERR "dma_set_mask_and_coherent fail %p\n", pdev);
   748			goto out_release_region;
   749		}
   750		rc = -ENODEV;
   751	
   752		/* Enable PCIE Advanced Error Recovery (AER) if kernel supports */
   753		pci_enable_pcie_error_reporting(pdev);
   754	
   755		bfad->pci_bar0_kva = pci_iomap(pdev, 0, pci_resource_len(pdev, 0));
   756		bfad->pci_bar2_kva = pci_iomap(pdev, 2, pci_resource_len(pdev, 2));
   757	
   758		if (bfad->pci_bar0_kva == NULL) {
   759			printk(KERN_ERR "Fail to map bar0\n");
   760			goto out_release_region;
   761		}
   762	
   763		bfad->hal_pcidev.pci_slot = PCI_SLOT(pdev->devfn);
   764		bfad->hal_pcidev.pci_func = PCI_FUNC(pdev->devfn);
   765		bfad->hal_pcidev.pci_bar_kva = bfad->pci_bar0_kva;
   766		bfad->hal_pcidev.device_id = pdev->device;
   767		bfad->hal_pcidev.ssid = pdev->subsystem_device;
   768		bfad->pci_name = pci_name(pdev);
   769	
   770		bfad->pci_attr.vendor_id = pdev->vendor;
   771		bfad->pci_attr.device_id = pdev->device;
   772		bfad->pci_attr.ssid = pdev->subsystem_device;
   773		bfad->pci_attr.ssvid = pdev->subsystem_vendor;
   774		bfad->pci_attr.pcifn = PCI_FUNC(pdev->devfn);
   775	
   776		bfad->pcidev = pdev;
   777	
   778		/* Adjust PCIe Maximum Read Request Size */
   779		if (pci_is_pcie(pdev) && pcie_max_read_reqsz) {
   780			if (pcie_max_read_reqsz >= 128 &&
   781			    pcie_max_read_reqsz <= 4096 &&
   782			    is_power_of_2(pcie_max_read_reqsz)) {
   783				int max_rq = pcie_get_readrq(pdev);
   784				printk(KERN_WARNING "BFA[%s]: "
   785					"pcie_max_read_request_size is %d, "
   786					"reset to %d\n", bfad->pci_name, max_rq,
   787					pcie_max_read_reqsz);
   788				pcie_set_readrq(pdev, pcie_max_read_reqsz);
   789			} else {
   790				printk(KERN_WARNING "BFA[%s]: invalid "
   791				       "pcie_max_read_request_size %d ignored\n",
   792				       bfad->pci_name, pcie_max_read_reqsz);
   793			}
   794		}
   795	
   796		pci_save_state(pdev);
   797	
   798		return 0;
   799	
   800	out_release_region:
   801		pci_release_regions(pdev);
   802	out_disable_device:
   803		pci_disable_device(pdev);
   804	out:
   805		return rc;
   806	}
   807	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56291 bytes --]

Re: [PATCH 3/4] bfa: fix calls to dma_set_mask_and_coherent()

[Dan Carpenter]

Hi Hannes,

url:    https://github.com/0day-ci/linux/commits/Hannes-Reinecke/scsi-fixup-dma_set_mask_and_coherent-calls/20190214-044535
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next

smatch warnings:
drivers/scsi/bfa/bfad.c:805 bfad_pci_init() error: uninitialized symbol 'rc'.

# https://github.com/0day-ci/linux/commit/48d00fbe6a39ffd9e97f505206b4394efb9803bc
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 48d00fbe6a39ffd9e97f505206b4394efb9803bc
vim +/rc +805 drivers/scsi/bfa/bfad.c

7725ccfda Jing Huang        2009-09-23  726  
7725ccfda Jing Huang        2009-09-23  727  int
7725ccfda Jing Huang        2009-09-23  728  bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
7725ccfda Jing Huang        2009-09-23  729  {
48d00fbe6 Hannes Reinecke   2019-02-13  730  	int rc;
7725ccfda Jing Huang        2009-09-23  731  
7725ccfda Jing Huang        2009-09-23  732  	if (pci_enable_device(pdev)) {
a36c61f90 Krishna Gudipati  2010-09-15  733  		printk(KERN_ERR "pci_enable_device fail %p\n", pdev);
7725ccfda Jing Huang        2009-09-23  734  		goto out;
                                                        ^^^^^^^^^

7725ccfda Jing Huang        2009-09-23  735  	}
7725ccfda Jing Huang        2009-09-23  736  
7725ccfda Jing Huang        2009-09-23  737  	if (pci_request_regions(pdev, BFAD_DRIVER_NAME))
7725ccfda Jing Huang        2009-09-23  738  		goto out_disable_device;
                                                        ^^^^^^^^^^^^^^^^^^^^^^^^
7725ccfda Jing Huang        2009-09-23  739  
7725ccfda Jing Huang        2009-09-23  740  	pci_set_master(pdev);
7725ccfda Jing Huang        2009-09-23  741  
48d00fbe6 Hannes Reinecke   2019-02-13  742  	rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
48d00fbe6 Hannes Reinecke   2019-02-13  743  	if (rc)
48d00fbe6 Hannes Reinecke   2019-02-13  744  		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
48d00fbe6 Hannes Reinecke   2019-02-13  745  
48d00fbe6 Hannes Reinecke   2019-02-13  746  	if (rc) {
a69b08002 Christoph Hellwig 2018-10-18  747  		printk(KERN_ERR "dma_set_mask_and_coherent fail %p\n", pdev);
7725ccfda Jing Huang        2009-09-23  748  		goto out_release_region;
7725ccfda Jing Huang        2009-09-23  749  	}
48d00fbe6 Hannes Reinecke   2019-02-13  750  	rc = -ENODEV;
7725ccfda Jing Huang        2009-09-23  751  
881c1b3c4 Krishna Gudipati  2012-08-22  752  	/* Enable PCIE Advanced Error Recovery (AER) if kernel supports */
881c1b3c4 Krishna Gudipati  2012-08-22  753  	pci_enable_pcie_error_reporting(pdev);
881c1b3c4 Krishna Gudipati  2012-08-22  754  
b3522f08e Jing Huang        2010-03-19  755  	bfad->pci_bar0_kva = pci_iomap(pdev, 0, pci_resource_len(pdev, 0));
111892082 Krishna Gudipati  2011-06-13  756  	bfad->pci_bar2_kva = pci_iomap(pdev, 2, pci_resource_len(pdev, 2));
7725ccfda Jing Huang        2009-09-23  757  
7725ccfda Jing Huang        2009-09-23  758  	if (bfad->pci_bar0_kva == NULL) {
a36c61f90 Krishna Gudipati  2010-09-15  759  		printk(KERN_ERR "Fail to map bar0\n");
7725ccfda Jing Huang        2009-09-23  760  		goto out_release_region;
7725ccfda Jing Huang        2009-09-23  761  	}
7725ccfda Jing Huang        2009-09-23  762  
7725ccfda Jing Huang        2009-09-23  763  	bfad->hal_pcidev.pci_slot = PCI_SLOT(pdev->devfn);
7725ccfda Jing Huang        2009-09-23  764  	bfad->hal_pcidev.pci_func = PCI_FUNC(pdev->devfn);
7725ccfda Jing Huang        2009-09-23  765  	bfad->hal_pcidev.pci_bar_kva = bfad->pci_bar0_kva;
7725ccfda Jing Huang        2009-09-23  766  	bfad->hal_pcidev.device_id = pdev->device;
1a4d8e1bd Krishna Gudipati  2011-06-24  767  	bfad->hal_pcidev.ssid = pdev->subsystem_device;
7725ccfda Jing Huang        2009-09-23  768  	bfad->pci_name = pci_name(pdev);
7725ccfda Jing Huang        2009-09-23  769  
7725ccfda Jing Huang        2009-09-23  770  	bfad->pci_attr.vendor_id = pdev->vendor;
7725ccfda Jing Huang        2009-09-23  771  	bfad->pci_attr.device_id = pdev->device;
7725ccfda Jing Huang        2009-09-23  772  	bfad->pci_attr.ssid = pdev->subsystem_device;
7725ccfda Jing Huang        2009-09-23  773  	bfad->pci_attr.ssvid = pdev->subsystem_vendor;
7725ccfda Jing Huang        2009-09-23  774  	bfad->pci_attr.pcifn = PCI_FUNC(pdev->devfn);
7725ccfda Jing Huang        2009-09-23  775  
7725ccfda Jing Huang        2009-09-23  776  	bfad->pcidev = pdev;
a36c61f90 Krishna Gudipati  2010-09-15  777  
a36c61f90 Krishna Gudipati  2010-09-15  778  	/* Adjust PCIe Maximum Read Request Size */
c0102c00d Yijing Wang       2013-09-05  779  	if (pci_is_pcie(pdev) && pcie_max_read_reqsz) {
c0102c00d Yijing Wang       2013-09-05  780  		if (pcie_max_read_reqsz >= 128 &&
c0102c00d Yijing Wang       2013-09-05  781  		    pcie_max_read_reqsz <= 4096 &&
c0102c00d Yijing Wang       2013-09-05  782  		    is_power_of_2(pcie_max_read_reqsz)) {
c0102c00d Yijing Wang       2013-09-05  783  			int max_rq = pcie_get_readrq(pdev);
a36c61f90 Krishna Gudipati  2010-09-15  784  			printk(KERN_WARNING "BFA[%s]: "
a36c61f90 Krishna Gudipati  2010-09-15  785  				"pcie_max_read_request_size is %d, "
c0102c00d Yijing Wang       2013-09-05  786  				"reset to %d\n", bfad->pci_name, max_rq,
a36c61f90 Krishna Gudipati  2010-09-15  787  				pcie_max_read_reqsz);
c0102c00d Yijing Wang       2013-09-05  788  			pcie_set_readrq(pdev, pcie_max_read_reqsz);
c0102c00d Yijing Wang       2013-09-05  789  		} else {
c0102c00d Yijing Wang       2013-09-05  790  			printk(KERN_WARNING "BFA[%s]: invalid "
c0102c00d Yijing Wang       2013-09-05  791  			       "pcie_max_read_request_size %d ignored\n",
c0102c00d Yijing Wang       2013-09-05  792  			       bfad->pci_name, pcie_max_read_reqsz);
a36c61f90 Krishna Gudipati  2010-09-15  793  		}
a36c61f90 Krishna Gudipati  2010-09-15  794  	}
a36c61f90 Krishna Gudipati  2010-09-15  795  
881c1b3c4 Krishna Gudipati  2012-08-22  796  	pci_save_state(pdev);
881c1b3c4 Krishna Gudipati  2012-08-22  797  
7725ccfda Jing Huang        2009-09-23  798  	return 0;
7725ccfda Jing Huang        2009-09-23  799  
7725ccfda Jing Huang        2009-09-23  800  out_release_region:
7725ccfda Jing Huang        2009-09-23  801  	pci_release_regions(pdev);
7725ccfda Jing Huang        2009-09-23  802  out_disable_device:
7725ccfda Jing Huang        2009-09-23  803  	pci_disable_device(pdev);
7725ccfda Jing Huang        2009-09-23  804  out:
7725ccfda Jing Huang        2009-09-23 @805  	return rc;
7725ccfda Jing Huang        2009-09-23  806  }
7725ccfda Jing Huang        2009-09-23  807  

:::::: The code at line 805 was first introduced by commit
:::::: 7725ccfda59715ecf8f99e3b520a0b84cc2ea79e [SCSI] bfa: Brocade BFA FC SCSI driver

:::::: TO: Jing Huang <huangj@brocade.com>
:::::: CC: James Bottomley <James.Bottomley@suse.de>

[PATCH 4/4] hisi_sas: fix calls to dma_set_mask_and_coherent()

[Hannes Reinecke]

The change to use dma_set_mask_and_coherent() incorrectly made a second
call with the 32 bit DMA mask value when the call with the 64 bit DMA
mask value succeeded.  This resulted in FC connections failing due
to corrupted data buffers, and various other SCSI/FCP I/O errors.

Fixes: e4db40e7a1a2 ("scsi: hisi_sas: use dma_set_mask_and_coherent")
Cc: <stable@vger.kernel.org>
Suggested-by: Ewan D. Milne <emilne@redhat.com>
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index eed7fc5b3389..bc17fa0d8375 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -2323,6 +2323,7 @@ static struct Scsi_Host *hisi_sas_shost_alloc(struct platform_device *pdev,
 	struct Scsi_Host *shost;
 	struct hisi_hba *hisi_hba;
 	struct device *dev = &pdev->dev;
+	int error;
 
 	shost = scsi_host_alloc(hw->sht, sizeof(*hisi_hba));
 	if (!shost) {
@@ -2343,8 +2344,11 @@ static struct Scsi_Host *hisi_sas_shost_alloc(struct platform_device *pdev,
 	if (hisi_sas_get_fw_info(hisi_hba) < 0)
 		goto err_out;
 
-	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)) &&
-	    dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
+	error = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	if (error)
+		error = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+
+	if (error) {
 		dev_err(dev, "No usable DMA addressing method\n");
 		goto err_out;
 	}
-- 
2.16.4

Re: [PATCH 4/4] hisi_sas: fix calls to dma_set_mask_and_coherent()

[John Garry]

On 13/02/2019 11:42, Hannes Reinecke wrote:
> The change to use dma_set_mask_and_coherent() incorrectly made a second
> call with the 32 bit DMA mask value when the call with the 64 bit DMA
> mask value succeeded.  This resulted in FC connections failing due
> to corrupted data buffers, and various other SCSI/FCP I/O errors.
>
> Fixes: e4db40e7a1a2 ("scsi: hisi_sas: use dma_set_mask_and_coherent")
> Cc: <stable@vger.kernel.org>
> Suggested-by: Ewan D. Milne <emilne@redhat.com>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
> index eed7fc5b3389..bc17fa0d8375 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> @@ -2323,6 +2323,7 @@ static struct Scsi_Host *hisi_sas_shost_alloc(struct platform_device *pdev,
>  	struct Scsi_Host *shost;
>  	struct hisi_hba *hisi_hba;
>  	struct device *dev = &pdev->dev;
> +	int error;
>
>  	shost = scsi_host_alloc(hw->sht, sizeof(*hisi_hba));
>  	if (!shost) {
> @@ -2343,8 +2344,11 @@ static struct Scsi_Host *hisi_sas_shost_alloc(struct platform_device *pdev,
>  	if (hisi_sas_get_fw_info(hisi_hba) < 0)
>  		goto err_out;
>
> -	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)) &&
> -	    dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {

Aren't we supposed to be fixing hisi_sas_v3_probe()? We have a path for 
platform device probe and a path for pci device probe. You're changing 
the platform device probe, which is currently ok.

I meant to unify all this. Now would be a good time, as it's not a fix. 
Can do it later.

Thanks,
John

> +	error = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +	if (error)
> +		error = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +
> +	if (error) {
>  		dev_err(dev, "No usable DMA addressing method\n");
>  		goto err_out;
>  	}
>

Re: [PATCH 4/4] hisi_sas: fix calls to dma_set_mask_and_coherent()

[Ewan D. Milne]

See comment below re: the patch description.  The
code itself looks fine.

-Ewan

On Wed, 2019-02-13 at 12:42 +0100, Hannes Reinecke wrote:
> The change to use dma_set_mask_and_coherent() incorrectly made a second
> call with the 32 bit DMA mask value when the call with the 64 bit DMA
> mask value succeeded.  This resulted in FC connections failing due
                         -------------------------------------------
> to corrupted data buffers, and various other SCSI/FCP I/O errors.
  -----------------------------------------------------------------

The last sentence should be removed from the patch description.

> 
> Fixes: e4db40e7a1a2 ("scsi: hisi_sas: use dma_set_mask_and_coherent")
> Cc: <stable@vger.kernel.org>
> Suggested-by: Ewan D. Milne <emilne@redhat.com>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
> index eed7fc5b3389..bc17fa0d8375 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> @@ -2323,6 +2323,7 @@ static struct Scsi_Host *hisi_sas_shost_alloc(struct platform_device *pdev,
>  	struct Scsi_Host *shost;
>  	struct hisi_hba *hisi_hba;
>  	struct device *dev = &pdev->dev;
> +	int error;
>  
>  	shost = scsi_host_alloc(hw->sht, sizeof(*hisi_hba));
>  	if (!shost) {
> @@ -2343,8 +2344,11 @@ static struct Scsi_Host *hisi_sas_shost_alloc(struct platform_device *pdev,
>  	if (hisi_sas_get_fw_info(hisi_hba) < 0)
>  		goto err_out;
>  
> -	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)) &&
> -	    dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
> +	error = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +	if (error)
> +		error = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +
> +	if (error) {
>  		dev_err(dev, "No usable DMA addressing method\n");
>  		goto err_out;
>  	}