[PATCH 0/1] scsi: qla1280.c

[PATCH 0/1] scsi: qla1280.c

[Magnus Lindholm]

To prevent file system corruption on 32-bit ISP10x0 cards in 64-bit enabled systems
while maintaing the possibility to run other qlogic cards in 64-bit mode:

 - Explicitly set enable_64bit_addressing flag if QLA_64BIT_PTR is
   defined and card is in a 64-bit slot.
 - Add information about chip hardware revision and pci-slot width to
   driver information string
 - For QLA1040, limit DMA_BIT_MASK to 32 bits for 32-bit cards

[PATCH 1/1] qla1280.c set DMA_BIT_MASK from bus width

[Magnus Lindholm]

Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
 drivers/scsi/qla1280.c | 112 ++++++++++++++++++++++++++++++-----------
 1 file changed, 84 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
index 8958547ac111..70409a140461 100644
--- a/drivers/scsi/qla1280.c
+++ b/drivers/scsi/qla1280.c
@@ -8,9 +8,15 @@
 * Copyright (C) 2003-2004 Christoph Hellwig
 *
 ******************************************************************************/
-#define QLA1280_VERSION      "3.27.1"
+#define QLA1280_VERSION      "3.27.2"
 /*****************************************************************************
     Revision History:
+    Rev  3.27.2, October 28, 2024, Magnus Lindholm
+	- Explicitly set enable_64bit_addressing flag if QLA_64BIT_PTR is
+	  defined and card is in a 64-bit slot.
+	- Add information about chip hardware revision and pci-slot width to
+	  driver information string
+	- For QLA1040, limit DMA_BIT_MASK to 32 bits for 32-bit cards
     Rev  3.27.1, February 8, 2010, Michael Reed
 	- Retain firmware image for error recovery.
     Rev  3.27, February 10, 2009, Michael Reed
@@ -368,6 +374,7 @@
 
 #define	MEMORY_MAPPED_IO	1
 
+
 #include "qla1280.h"
 
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
@@ -655,18 +662,51 @@ qla1280_info(struct Scsi_Host *host)
 {
 	static char qla1280_scsi_name_buffer[125];
 	char *bp;
+	char hwrev = ' ';
+	int bits;
 	struct scsi_qla_host *ha;
 	struct qla_boards *bdp;
+	struct device_reg __iomem *reg;
 
 	bp = &qla1280_scsi_name_buffer[0];
 	ha = (struct scsi_qla_host *)host->hostdata;
+	reg = ha->iobase;
 	bdp = &ql1280_board_tbl[ha->devnum];
 	memset(bp, 0, sizeof(qla1280_scsi_name_buffer));
 
+
+	if (IS_ISP1040(ha))
+		switch (ha->revision) {
+		case 1:
+			hwrev = ' ';
+			break;
+		case 2:
+			hwrev = 'A';
+			break;
+		case 3:
+			hwrev = ' ';
+			break;
+		case 4:
+			hwrev = 'A';
+			break;
+		case 5:
+			hwrev = 'B';
+			break;
+		case 6:
+			hwrev = 'C';
+			break;
+		default:
+			hwrev = '?';
+			break;
+	}
+	if (RD_REG_WORD(&reg->istatus) & PCI_64BIT_SLOT)
+		bits = 64;
+	else
+		bits = 32;
 	sprintf (bp,
-		 "QLogic %s PCI to SCSI Host Adapter\n"
+		 "QLogic %s%c %d-bit PCI to SCSI Host Adapter\n"
 		 "       Firmware version: %2d.%02d.%02d, Driver version %s",
-		 &bdp->name[0], ha->fwver1, ha->fwver2, ha->fwver3,
+		 &bdp->name[0], hwrev, bits, ha->fwver1, ha->fwver2, ha->fwver3,
 		 QLA1280_VERSION);
 	return bp;
 }
@@ -2168,6 +2208,13 @@ qla1280_nvram_config(struct scsi_qla_host *ha)
 	} else {
 		qla1280_set_defaults(ha);
 	}
+#ifdef QLA_64BIT_PTR
+	/* set flag in nvram if we're using 64 bit addressing */
+	if (RD_REG_WORD(&reg->istatus) & PCI_64BIT_SLOT)
+		nv->cntr_flags_1.enable_64bit_addressing = 1;
+	else
+		nv->cntr_flags_1.enable_64bit_addressing = 0;
+#endif
 
 	qla1280_print_settings(nv);
 
@@ -2177,9 +2224,9 @@ qla1280_nvram_config(struct scsi_qla_host *ha)
 
 	if (IS_ISP1040(ha)) {
 		uint16_t hwrev, cfg1, cdma_conf;
-
 		hwrev = RD_REG_WORD(&reg->cfg_0) & ISP_CFG0_HWMSK;
 
+		ha->revision = hwrev;
 		cfg1 = RD_REG_WORD(&reg->cfg_1) & ~(BIT_4 | BIT_5 | BIT_6);
 		cdma_conf = RD_REG_WORD(&reg->cdma_cfg);
 
@@ -4139,9 +4186,11 @@ static int
 qla1280_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	int devnum = id->driver_data;
+	u64 mask;
 	struct qla_boards *bdp = &ql1280_board_tbl[devnum];
 	struct Scsi_Host *host;
 	struct scsi_qla_host *ha;
+	struct device_reg __iomem *reg;
 	int error = -ENODEV;
 
 	/* Bypass all AMI SUBSYS VENDOR IDs */
@@ -4176,9 +4225,38 @@ qla1280_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	ha->pdev = pdev;
 	ha->devnum = devnum;	/* specifies microcode load address */
 
+
+
+#if MEMORY_MAPPED_IO
+	ha->mmpbase = pci_ioremap_bar(ha->pdev, 1);
+	if (!ha->mmpbase) {
+		printk(KERN_INFO "qla1280: Unable to map I/O memory\n");
+	goto error_free_response_ring;
+	}
+
+	host->base = (unsigned long)ha->mmpbase;
+	ha->iobase = (struct device_reg __iomem *)ha->mmpbase;
+#else
+	host->io_port = pci_resource_start(ha->pdev, 0);
+	if (!request_region(host->io_port, 0xff, "qla1280")) {
+		printk(KERN_INFO "qla1280: Failed to reserve i/o region "
+				 "0x%04lx-0x%04lx - already in use\n",
+		host->io_port, host->io_port + 0xff);
+	goto error_free_response_ring;
+	}
+
+	ha->iobase = (struct device_reg *)host->io_port;
+#endif
+
+
 #ifdef QLA_64BIT_PTR
-	if (dma_set_mask_and_coherent(&ha->pdev->dev, DMA_BIT_MASK(64))) {
-		if (dma_set_mask(&ha->pdev->dev, DMA_BIT_MASK(32))) {
+	reg = ha->iobase;
+	if (RD_REG_WORD(&reg->istatus) & PCI_64BIT_SLOT)
+		mask = DMA_BIT_MASK(64);
+	else
+		mask = DMA_BIT_MASK(32);
+	if (dma_set_mask_and_coherent(&ha->pdev->dev, mask)) {
+		if (dma_set_mask(&ha->pdev->dev, mask)) {
 			printk(KERN_WARNING "scsi(%li): Unable to set a "
 			       "suitable DMA mask - aborting\n", ha->host_no);
 			error = -ENODEV;
@@ -4225,28 +4303,6 @@ qla1280_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	host->unique_id = host->host_no;
 
 	error = -ENODEV;
-
-#if MEMORY_MAPPED_IO
-	ha->mmpbase = pci_ioremap_bar(ha->pdev, 1);
-	if (!ha->mmpbase) {
-		printk(KERN_INFO "qla1280: Unable to map I/O memory\n");
-		goto error_free_response_ring;
-	}
-
-	host->base = (unsigned long)ha->mmpbase;
-	ha->iobase = (struct device_reg __iomem *)ha->mmpbase;
-#else
-	host->io_port = pci_resource_start(ha->pdev, 0);
-	if (!request_region(host->io_port, 0xff, "qla1280")) {
-		printk(KERN_INFO "qla1280: Failed to reserve i/o region "
-				 "0x%04lx-0x%04lx - already in use\n",
-		       host->io_port, host->io_port + 0xff);
-		goto error_free_response_ring;
-	}
-
-	ha->iobase = (struct device_reg *)host->io_port;
-#endif
-
 	INIT_LIST_HEAD(&ha->done_q);
 
 	/* Disable ISP interrupts. */
-- 
2.47.0

Re: [PATCH 1/1] qla1280.c set DMA_BIT_MASK from bus width

[Maciej W. Rozycki]

On Tue, 29 Oct 2024, Magnus Lindholm wrote:

> Signed-off-by: Magnus Lindholm <linmag7@gmail.com>

 You'll need some text in your change description for your patch to be 
accepted.

> diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
> index 8958547ac111..70409a140461 100644
> --- a/drivers/scsi/qla1280.c
> +++ b/drivers/scsi/qla1280.c
> @@ -368,6 +374,7 @@
>  
>  #define	MEMORY_MAPPED_IO	1
>  
> +

 Please don't add an extra line.

> @@ -655,18 +662,51 @@ qla1280_info(struct Scsi_Host *host)
>  {
>  	static char qla1280_scsi_name_buffer[125];
>  	char *bp;
> +	char hwrev = ' ';
> +	int bits;
>  	struct scsi_qla_host *ha;
>  	struct qla_boards *bdp;
> +	struct device_reg __iomem *reg;
>  
>  	bp = &qla1280_scsi_name_buffer[0];
>  	ha = (struct scsi_qla_host *)host->hostdata;
> +	reg = ha->iobase;
>  	bdp = &ql1280_board_tbl[ha->devnum];
>  	memset(bp, 0, sizeof(qla1280_scsi_name_buffer));
>  
> +

 Likewise.

> +	if (IS_ISP1040(ha))
> +		switch (ha->revision) {
> +		case 1:
> +			hwrev = ' ';
> +			break;
> +		case 2:
> +			hwrev = 'A';
> +			break;
> +		case 3:
> +			hwrev = ' ';
> +			break;
> +		case 4:
> +			hwrev = 'A';
> +			break;
> +		case 5:
> +			hwrev = 'B';
> +			break;
> +		case 6:
> +			hwrev = 'C';
> +			break;
> +		default:
> +			hwrev = '?';
> +			break;
> +	}

 Currently QLA1040 is always printed even for ISP1020 devices, so I think 
adding a revision to that will only be confusing.  Can this reporting be 
improved by any chance?

> @@ -2177,9 +2224,9 @@ qla1280_nvram_config(struct scsi_qla_host *ha)
>  
>  	if (IS_ISP1040(ha)) {
>  		uint16_t hwrev, cfg1, cdma_conf;
> -

 Please don't remove the line.

>  		hwrev = RD_REG_WORD(&reg->cfg_0) & ISP_CFG0_HWMSK;
>  
> +		ha->revision = hwrev;

 Notice that masking with ISP_CFG0_HWMSK clobbers the revision.  It does 
not matter for existing code, but it breaks your reporting improvement.

 I'm leaving the rest for the time being as you'll need to change the DMA 
mask selection logic as per the other thread.  Overall you'll have to 
split your change into a patch series, with the DMA mask fix and reporting 
improvements each as an individual self-contained update.  This is how we 
do development in principle, but in this particular case the fix also asks 
for being backported, as it addresses data corruption, so it has to be on 
its own.

 NB please run updated patches through scripts/get_maintainer.pl for the 
complete list of recipients to send to.

  Maciej