[PATCH v2 0/5] Fix mpt3sas driver sparse warnings

[PATCH v2 0/5] Fix mpt3sas driver sparse warnings

[Damien Le Moal]

This series fix (remove) all sparse warnings generated when compiling
the mpt3sas driver. All warnings are related to __iomem access and
endianness.

The series was tested on top of Martin's 5.18/scsi-staging branch with a
9400-8i HBA with direct attached iSAS and SATA drives. The fixes need
careful review by the maintainers as there is no documentation clearly
explaning the proper endianness of the values touched.

Changes from v1:
* Reworked patch 1 to remove the TaskMID field type change and simplify
  _ctl_set_task_mid() code.

Damien Le Moal (5):
  scsi: mpt3sas: fix _ctl_set_task_mid() TaskMID check
  scsi: mpt3sas: Fix writel() use
  scsi: mpt3sas: fix ioc->base_readl() use
  scsi: mpt3sas: fix event callback log_code value handling
  scsi: mpt3sas: fix adapter replyPostRegisterIndex handling

 drivers/scsi/mpt3sas/mpt3sas_base.c  | 60 ++++++++++++++++------------
 drivers/scsi/mpt3sas/mpt3sas_ctl.c   | 11 ++---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |  6 +--
 3 files changed, 43 insertions(+), 34 deletions(-)

-- 
2.35.1

[PATCH v2 1/5] scsi: mpt3sas: fix _ctl_set_task_mid() TaskMID check

[Damien Le Moal]

The TaskMID field of struct Mpi2SCSITaskManagementRequest_t is a 16-bits
little endian value. Fix the search loop in _ctl_set_task_mid() to add
a cpu_to_le16() conversion before checking the value of TaskMID to avoid
sparse warnings. While at it, simplify the search loop code to remove an
unnecessarily complicated if condition.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/scsi/mpt3sas/mpt3sas_ctl.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index d92ca140d298..84c87c2c3e7e 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -578,7 +578,7 @@ static int
 _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command *karg,
 	Mpi2SCSITaskManagementRequest_t *tm_request)
 {
-	u8 found = 0;
+	bool found = false;
 	u16 smid;
 	u16 handle;
 	struct scsi_cmnd *scmd;
@@ -600,6 +600,7 @@ _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command *karg,
 	handle = le16_to_cpu(tm_request->DevHandle);
 	for (smid = ioc->scsiio_depth; smid && !found; smid--) {
 		struct scsiio_tracker *st;
+		__le16 task_mid;
 
 		scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
 		if (!scmd)
@@ -618,10 +619,10 @@ _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command *karg,
 		 * first outstanding smid will be picked up.  Otherwise,
 		 * targeted smid will be the one.
 		 */
-		if (!tm_request->TaskMID || tm_request->TaskMID == st->smid) {
-			tm_request->TaskMID = cpu_to_le16(st->smid);
-			found = 1;
-		}
+		task_mid = cpu_to_le16(st->smid);
+		if (!tm_request->TaskMID)
+			tm_request->TaskMID = task_mid;
+		found = tm_request->TaskMID == task_mid;
 	}
 
 	if (!found) {
-- 
2.35.1

[PATCH v2 2/5] scsi: mpt3sas: Fix writel() use

[Damien Le Moal]

writel() internally execute cpu_to_le32() to convert the vale being
wrritten to little endian. The caller should thus not use this
conversion function for the value passed to writel(). Remove the
cpu_to_le32() calls in _base_put_smid_scsi_io_atomic(),
_base_put_smid_fast_path_atomic(), _base_put_smid_hi_priority_atomic()
_base_put_smid_default_atomic() amd _base_handshake_req_reply_wait().

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 511726f92d9a..6ebdfedd84f5 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4323,7 +4323,7 @@ _base_put_smid_scsi_io_atomic(struct MPT3SAS_ADAPTER *ioc, u16 smid,
 	descriptor.MSIxIndex = _base_set_and_get_msix_index(ioc, smid);
 	descriptor.SMID = cpu_to_le16(smid);
 
-	writel(cpu_to_le32(*request), &ioc->chip->AtomicRequestDescriptorPost);
+	writel(*request, &ioc->chip->AtomicRequestDescriptorPost);
 }
 
 /**
@@ -4345,7 +4345,7 @@ _base_put_smid_fast_path_atomic(struct MPT3SAS_ADAPTER *ioc, u16 smid,
 	descriptor.MSIxIndex = _base_set_and_get_msix_index(ioc, smid);
 	descriptor.SMID = cpu_to_le16(smid);
 
-	writel(cpu_to_le32(*request), &ioc->chip->AtomicRequestDescriptorPost);
+	writel(*request, &ioc->chip->AtomicRequestDescriptorPost);
 }
 
 /**
@@ -4368,7 +4368,7 @@ _base_put_smid_hi_priority_atomic(struct MPT3SAS_ADAPTER *ioc, u16 smid,
 	descriptor.MSIxIndex = msix_task;
 	descriptor.SMID = cpu_to_le16(smid);
 
-	writel(cpu_to_le32(*request), &ioc->chip->AtomicRequestDescriptorPost);
+	writel(*request, &ioc->chip->AtomicRequestDescriptorPost);
 }
 
 /**
@@ -4389,7 +4389,7 @@ _base_put_smid_default_atomic(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 	descriptor.MSIxIndex = _base_set_and_get_msix_index(ioc, smid);
 	descriptor.SMID = cpu_to_le16(smid);
 
-	writel(cpu_to_le32(*request), &ioc->chip->AtomicRequestDescriptorPost);
+	writel(*request, &ioc->chip->AtomicRequestDescriptorPost);
 }
 
 /**
@@ -6906,7 +6906,7 @@ _base_handshake_req_reply_wait(struct MPT3SAS_ADAPTER *ioc, int request_bytes,
 
 	/* send message 32-bits at a time */
 	for (i = 0, failed = 0; i < request_bytes/4 && !failed; i++) {
-		writel(cpu_to_le32(request[i]), &ioc->chip->Doorbell);
+		writel(request[i], &ioc->chip->Doorbell);
 		if ((_base_wait_for_doorbell_ack(ioc, 5)))
 			failed = 1;
 	}
-- 
2.35.1

[PATCH v2 3/5] scsi: mpt3sas: fix ioc->base_readl() use

[Damien Le Moal]

The functions _base_readl_aero() and _base_readl() used for an adapter
base_readl() method are implemented using a regular readl() call which
performs internally a conversion to CPU endianness (le32_to_cpu()) of
the values read. The users of the ioc base_readl() method should thus
not convert again the values read using le16_to_cpu().
Fixing this removes sparse warnings.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 6ebdfedd84f5..5efe4bd186db 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -6925,16 +6925,16 @@ _base_handshake_req_reply_wait(struct MPT3SAS_ADAPTER *ioc, int request_bytes,
 	}
 
 	/* read the first two 16-bits, it gives the total length of the reply */
-	reply[0] = le16_to_cpu(ioc->base_readl(&ioc->chip->Doorbell)
-	    & MPI2_DOORBELL_DATA_MASK);
+	reply[0] = ioc->base_readl(&ioc->chip->Doorbell)
+		& MPI2_DOORBELL_DATA_MASK;
 	writel(0, &ioc->chip->HostInterruptStatus);
 	if ((_base_wait_for_doorbell_int(ioc, 5))) {
 		ioc_err(ioc, "doorbell handshake int failed (line=%d)\n",
 			__LINE__);
 		return -EFAULT;
 	}
-	reply[1] = le16_to_cpu(ioc->base_readl(&ioc->chip->Doorbell)
-	    & MPI2_DOORBELL_DATA_MASK);
+	reply[1] = ioc->base_readl(&ioc->chip->Doorbell)
+		& MPI2_DOORBELL_DATA_MASK;
 	writel(0, &ioc->chip->HostInterruptStatus);
 
 	for (i = 2; i < default_reply->MsgLength * 2; i++)  {
@@ -6946,9 +6946,8 @@ _base_handshake_req_reply_wait(struct MPT3SAS_ADAPTER *ioc, int request_bytes,
 		if (i >=  reply_bytes/2) /* overflow case */
 			ioc->base_readl(&ioc->chip->Doorbell);
 		else
-			reply[i] = le16_to_cpu(
-			    ioc->base_readl(&ioc->chip->Doorbell)
-			    & MPI2_DOORBELL_DATA_MASK);
+			reply[i] = ioc->base_readl(&ioc->chip->Doorbell)
+				& MPI2_DOORBELL_DATA_MASK;
 		writel(0, &ioc->chip->HostInterruptStatus);
 	}
 
-- 
2.35.1

[PATCH v2 4/5] scsi: mpt3sas: fix event callback log_code value handling

[Damien Le Moal]

In mpt3sas_scsih_event_callback(), fix a sparse warning when testing the
event log code value by replacing the use of a pointer to the address
storing the event log code with a log code local variable . Doing so,
le32_to_cpu() is used when the log code value is assigned, avoiding a
sparse warning.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 00792767c620..a355bc145577 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -10926,20 +10926,20 @@ mpt3sas_scsih_event_callback(struct MPT3SAS_ADAPTER *ioc, u8 msix_index,
 	case MPI2_EVENT_LOG_ENTRY_ADDED:
 	{
 		Mpi2EventDataLogEntryAdded_t *log_entry;
-		u32 *log_code;
+		u32 log_code;
 
 		if (!ioc->is_warpdrive)
 			break;
 
 		log_entry = (Mpi2EventDataLogEntryAdded_t *)
 		    mpi_reply->EventData;
-		log_code = (u32 *)log_entry->LogData;
+		log_code = le32_to_cpu(*(__le32 *)log_entry->LogData);
 
 		if (le16_to_cpu(log_entry->LogEntryQualifier)
 		    != MPT2_WARPDRIVE_LOGENTRY)
 			break;
 
-		switch (le32_to_cpu(*log_code)) {
+		switch (log_code) {
 		case MPT2_WARPDRIVE_LC_SSDT:
 			ioc_warn(ioc, "WarpDrive Warning: IO Throttling has occurred in the WarpDrive subsystem. Check WarpDrive documentation for additional details.\n");
 			break;
-- 
2.35.1

[PATCH v2 5/5] scsi: mpt3sas: fix adapter replyPostRegisterIndex handling

[Damien Le Moal]

The replyPostRegisterIndex array of struct MPT3SAS_ADAPTER is regular
memory allocated from RAM, and not an IO mem resource. writel() should
thus not be used to assign values to the array entries. Replace the
writel() call modifying the array with direct assignements. This change
suppresses sparse warnings.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 37 ++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 5efe4bd186db..4caa719b4ef4 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1771,10 +1771,13 @@ _base_process_reply_queue(struct adapter_reply_queue *reply_q)
 		 */
 		if (completed_cmds >= ioc->thresh_hold) {
 			if (ioc->combined_reply_queue) {
-				writel(reply_q->reply_post_host_index |
-						((msix_index  & 7) <<
-						 MPI2_RPHI_MSIX_INDEX_SHIFT),
-				    ioc->replyPostRegisterIndex[msix_index/8]);
+				unsigned long idx =
+					reply_q->reply_post_host_index |
+					((msix_index  & 7) <<
+					 MPI2_RPHI_MSIX_INDEX_SHIFT);
+
+				ioc->replyPostRegisterIndex[msix_index/8] =
+					(resource_size_t *) idx;
 			} else {
 				writel(reply_q->reply_post_host_index |
 						(msix_index <<
@@ -1826,14 +1829,17 @@ _base_process_reply_queue(struct adapter_reply_queue *reply_q)
 	 * new reply host index value in ReplyPostIndex Field and msix_index
 	 * value in MSIxIndex field.
 	 */
-	if (ioc->combined_reply_queue)
-		writel(reply_q->reply_post_host_index | ((msix_index  & 7) <<
-			MPI2_RPHI_MSIX_INDEX_SHIFT),
-			ioc->replyPostRegisterIndex[msix_index/8]);
-	else
+	if (ioc->combined_reply_queue) {
+		unsigned long idx = reply_q->reply_post_host_index |
+			((msix_index  & 7) << MPI2_RPHI_MSIX_INDEX_SHIFT);
+
+		ioc->replyPostRegisterIndex[msix_index/8] =
+			(resource_size_t *) idx;
+	} else {
 		writel(reply_q->reply_post_host_index | (msix_index <<
 			MPI2_RPHI_MSIX_INDEX_SHIFT),
 			&ioc->chip->ReplyPostHostIndex);
+	}
 	atomic_dec(&reply_q->busy);
 	return completed_cmds;
 }
@@ -8095,14 +8101,17 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc)
 
 	/* initialize reply post host index */
 	list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
-		if (ioc->combined_reply_queue)
-			writel((reply_q->msix_index & 7)<<
-			   MPI2_RPHI_MSIX_INDEX_SHIFT,
-			   ioc->replyPostRegisterIndex[reply_q->msix_index/8]);
-		else
+		if (ioc->combined_reply_queue) {
+			unsigned long idx =(reply_q->msix_index & 7) <<
+				MPI2_RPHI_MSIX_INDEX_SHIFT;
+
+			ioc->replyPostRegisterIndex[reply_q->msix_index/8] =
+				(resource_size_t *) idx;
+		} else {
 			writel(reply_q->msix_index <<
 				MPI2_RPHI_MSIX_INDEX_SHIFT,
 				&ioc->chip->ReplyPostHostIndex);
+		}
 
 		if (!_base_is_controller_msix_enabled(ioc))
 			goto skip_init_reply_post_host_index;
-- 
2.35.1

Re: [PATCH v2 0/5] Fix mpt3sas driver sparse warnings

[Martin K. Petersen]

> This series fix (remove) all sparse warnings generated when compiling
> the mpt3sas driver. All warnings are related to __iomem access and
> endianness.
>
> The series was tested on top of Martin's 5.18/scsi-staging branch with
> a 9400-8i HBA with direct attached iSAS and SATA drives. The fixes
> need careful review by the maintainers as there is no documentation
> clearly explaning the proper endianness of the values touched.

Broadcom: Please review!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 0/5] Fix mpt3sas driver sparse warnings

[Sreekanth Reddy]

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

On Wed, Mar 2, 2022 at 9:54 AM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
>
> > This series fix (remove) all sparse warnings generated when compiling
> > the mpt3sas driver. All warnings are related to __iomem access and
> > endianness.
> >
> > The series was tested on top of Martin's 5.18/scsi-staging branch with
> > a 9400-8i HBA with direct attached iSAS and SATA drives. The fixes
> > need careful review by the maintainers as there is no documentation
> > clearly explaning the proper endianness of the values touched.
>
> Broadcom: Please review!

Sure Martin. I will complete the review by the end of this week.

>
> --
> Martin K. Petersen      Oracle Linux Engineering

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH v2 5/5] scsi: mpt3sas: fix adapter replyPostRegisterIndex handling

[Sreekanth Reddy]

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

On Fri, Feb 25, 2022 at 5:01 AM Damien Le Moal
<damien.lemoal@opensource.wdc.com> wrote:
>
> The replyPostRegisterIndex array of struct MPT3SAS_ADAPTER is regular
> memory allocated from RAM, and not an IO mem resource. writel() should
> thus not be used to assign values to the array entries. Replace the
> writel() call modifying the array with direct assignements. This change
> suppresses sparse warnings.

writel() is a must here.  replyPostRegisterIndex array is having the
iommu address.
and here the driver is writing the data to these iommu addresses retrieved from
replyPostRegisterIndex array.

Thanks,
Sreekanth

>
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 37 ++++++++++++++++++-----------
>  1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 5efe4bd186db..4caa719b4ef4 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -1771,10 +1771,13 @@ _base_process_reply_queue(struct adapter_reply_queue *reply_q)
>                  */
>                 if (completed_cmds >= ioc->thresh_hold) {
>                         if (ioc->combined_reply_queue) {
> -                               writel(reply_q->reply_post_host_index |
> -                                               ((msix_index  & 7) <<
> -                                                MPI2_RPHI_MSIX_INDEX_SHIFT),
> -                                   ioc->replyPostRegisterIndex[msix_index/8]);
> +                               unsigned long idx =
> +                                       reply_q->reply_post_host_index |
> +                                       ((msix_index  & 7) <<
> +                                        MPI2_RPHI_MSIX_INDEX_SHIFT);
> +
> +                               ioc->replyPostRegisterIndex[msix_index/8] =
> +                                       (resource_size_t *) idx;
>                         } else {
>                                 writel(reply_q->reply_post_host_index |
>                                                 (msix_index <<
> @@ -1826,14 +1829,17 @@ _base_process_reply_queue(struct adapter_reply_queue *reply_q)
>          * new reply host index value in ReplyPostIndex Field and msix_index
>          * value in MSIxIndex field.
>          */
> -       if (ioc->combined_reply_queue)
> -               writel(reply_q->reply_post_host_index | ((msix_index  & 7) <<
> -                       MPI2_RPHI_MSIX_INDEX_SHIFT),
> -                       ioc->replyPostRegisterIndex[msix_index/8]);
> -       else
> +       if (ioc->combined_reply_queue) {
> +               unsigned long idx = reply_q->reply_post_host_index |
> +                       ((msix_index  & 7) << MPI2_RPHI_MSIX_INDEX_SHIFT);
> +
> +               ioc->replyPostRegisterIndex[msix_index/8] =
> +                       (resource_size_t *) idx;
> +       } else {
>                 writel(reply_q->reply_post_host_index | (msix_index <<
>                         MPI2_RPHI_MSIX_INDEX_SHIFT),
>                         &ioc->chip->ReplyPostHostIndex);
> +       }
>         atomic_dec(&reply_q->busy);
>         return completed_cmds;
>  }
> @@ -8095,14 +8101,17 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc)
>
>         /* initialize reply post host index */
>         list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
> -               if (ioc->combined_reply_queue)
> -                       writel((reply_q->msix_index & 7)<<
> -                          MPI2_RPHI_MSIX_INDEX_SHIFT,
> -                          ioc->replyPostRegisterIndex[reply_q->msix_index/8]);
> -               else
> +               if (ioc->combined_reply_queue) {
> +                       unsigned long idx =(reply_q->msix_index & 7) <<
> +                               MPI2_RPHI_MSIX_INDEX_SHIFT;
> +
> +                       ioc->replyPostRegisterIndex[reply_q->msix_index/8] =
> +                               (resource_size_t *) idx;
> +               } else {
>                         writel(reply_q->msix_index <<
>                                 MPI2_RPHI_MSIX_INDEX_SHIFT,
>                                 &ioc->chip->ReplyPostHostIndex);
> +               }
>
>                 if (!_base_is_controller_msix_enabled(ioc))
>                         goto skip_init_reply_post_host_index;
> --
> 2.35.1
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH v2 5/5] scsi: mpt3sas: fix adapter replyPostRegisterIndex handling

[Damien Le Moal]

On 3/7/22 16:35, Sreekanth Reddy wrote:
> On Fri, Feb 25, 2022 at 5:01 AM Damien Le Moal
> <damien.lemoal@opensource.wdc.com> wrote:
>>
>> The replyPostRegisterIndex array of struct MPT3SAS_ADAPTER is regular
>> memory allocated from RAM, and not an IO mem resource. writel() should
>> thus not be used to assign values to the array entries. Replace the
>> writel() call modifying the array with direct assignements. This change
>> suppresses sparse warnings.
> 
> writel() is a must here.  replyPostRegisterIndex array is having the
> iommu address.
> and here the driver is writing the data to these iommu addresses retrieved from
> replyPostRegisterIndex array.

So the declaration of this array of wrong. it should be an array of
__iomem void * entries, not an array of resource_size_t * entries (which
by the way does not make sense to me since the use of the array is
clearly to store an address, not a pointer to an address).

How do you prefer this fixed ? I would rather not add local casts here
and fix the declaration of the replyPostRegisterIndex array.

> 
> Thanks,
> Sreekanth
> 
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_base.c | 37 ++++++++++++++++++-----------
>>  1 file changed, 23 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index 5efe4bd186db..4caa719b4ef4 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -1771,10 +1771,13 @@ _base_process_reply_queue(struct adapter_reply_queue *reply_q)
>>                  */
>>                 if (completed_cmds >= ioc->thresh_hold) {
>>                         if (ioc->combined_reply_queue) {
>> -                               writel(reply_q->reply_post_host_index |
>> -                                               ((msix_index  & 7) <<
>> -                                                MPI2_RPHI_MSIX_INDEX_SHIFT),
>> -                                   ioc->replyPostRegisterIndex[msix_index/8]);
>> +                               unsigned long idx =
>> +                                       reply_q->reply_post_host_index |
>> +                                       ((msix_index  & 7) <<
>> +                                        MPI2_RPHI_MSIX_INDEX_SHIFT);
>> +
>> +                               ioc->replyPostRegisterIndex[msix_index/8] =
>> +                                       (resource_size_t *) idx;
>>                         } else {
>>                                 writel(reply_q->reply_post_host_index |
>>                                                 (msix_index <<
>> @@ -1826,14 +1829,17 @@ _base_process_reply_queue(struct adapter_reply_queue *reply_q)
>>          * new reply host index value in ReplyPostIndex Field and msix_index
>>          * value in MSIxIndex field.
>>          */
>> -       if (ioc->combined_reply_queue)
>> -               writel(reply_q->reply_post_host_index | ((msix_index  & 7) <<
>> -                       MPI2_RPHI_MSIX_INDEX_SHIFT),
>> -                       ioc->replyPostRegisterIndex[msix_index/8]);
>> -       else
>> +       if (ioc->combined_reply_queue) {
>> +               unsigned long idx = reply_q->reply_post_host_index |
>> +                       ((msix_index  & 7) << MPI2_RPHI_MSIX_INDEX_SHIFT);
>> +
>> +               ioc->replyPostRegisterIndex[msix_index/8] =
>> +                       (resource_size_t *) idx;
>> +       } else {
>>                 writel(reply_q->reply_post_host_index | (msix_index <<
>>                         MPI2_RPHI_MSIX_INDEX_SHIFT),
>>                         &ioc->chip->ReplyPostHostIndex);
>> +       }
>>         atomic_dec(&reply_q->busy);
>>         return completed_cmds;
>>  }
>> @@ -8095,14 +8101,17 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc)
>>
>>         /* initialize reply post host index */
>>         list_for_each_entry(reply_q, &ioc->reply_queue_list, list) {
>> -               if (ioc->combined_reply_queue)
>> -                       writel((reply_q->msix_index & 7)<<
>> -                          MPI2_RPHI_MSIX_INDEX_SHIFT,
>> -                          ioc->replyPostRegisterIndex[reply_q->msix_index/8]);
>> -               else
>> +               if (ioc->combined_reply_queue) {
>> +                       unsigned long idx =(reply_q->msix_index & 7) <<
>> +                               MPI2_RPHI_MSIX_INDEX_SHIFT;
>> +
>> +                       ioc->replyPostRegisterIndex[reply_q->msix_index/8] =
>> +                               (resource_size_t *) idx;
>> +               } else {
>>                         writel(reply_q->msix_index <<
>>                                 MPI2_RPHI_MSIX_INDEX_SHIFT,
>>                                 &ioc->chip->ReplyPostHostIndex);
>> +               }
>>
>>                 if (!_base_is_controller_msix_enabled(ioc))
>>                         goto skip_init_reply_post_host_index;
>> --
>> 2.35.1
>>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2 5/5] scsi: mpt3sas: fix adapter replyPostRegisterIndex handling

[Damien Le Moal]

On 3/7/22 16:35, Sreekanth Reddy wrote:
> On Fri, Feb 25, 2022 at 5:01 AM Damien Le Moal
> <damien.lemoal@opensource.wdc.com> wrote:
>>
>> The replyPostRegisterIndex array of struct MPT3SAS_ADAPTER is regular
>> memory allocated from RAM, and not an IO mem resource. writel() should
>> thus not be used to assign values to the array entries. Replace the
>> writel() call modifying the array with direct assignements. This change
>> suppresses sparse warnings.
> 
> writel() is a must here.  replyPostRegisterIndex array is having the
> iommu address.
> and here the driver is writing the data to these iommu addresses retrieved from
> replyPostRegisterIndex array.

Fixed in v3. Please check.


-- 
Damien Le Moal
Western Digital Research