On 3/7/22 16:35, Sreekanth Reddy wrote:
> On Fri, Feb 25, 2022 at 5:01 AM Damien Le Moal
> <damien.lemoal@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
>> ---
>> 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
