Re: [PATCH] mpt2sas: Fall back to 64 bit coherent mask if 64 bit DMA / 32 bit coherent mask not supported

[Brian King <brking@linux.vnet.ibm.com>]

On 05/13/2015 03:56 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2015-05-13 at 22:14 +0530, Sreekanth Reddy wrote:
>> Hi Brain,
>>
>> We had a restriction from the firmware that the upper 32 bits of the
>> RDPQ pool entries must be same. So to limit this restriction we
>> initially set the consistent DMA mask to 32 and then after allocating
>> the RDPQ pools we changed the consistent DMA mask to 64 before
>> allocating remaining pools.
> 
> Brian, maybe we should just bite that bullet and implement that hybrid
> DMA ops I mentioned. I'll see if I can spare some time today to look
> at it. It would work as long as we never remove the legacy window using
> the DDW APIs (removing the legacy window is broken anyway for other
> reasons I think we discussed already).
> 
> As long as we have both the legacy window and the DDW available (or the
> legacy and bypass on "nv"), we can then route individual DMAs according
> to the corresponding applicable mask.
> 
> I'll try to come up with a patch but I'll need you to test it.

Sure. I can help with that.

Thanks,

Brian

> 
> Cheers,
> Ben.
> 
>> Regards,
>> Sreekanth
>>
>> On Wed, May 13, 2015 at 7:42 PM, Brian King <brking@linux.vnet.ibm.com> wrote:
>>> On 05/13/2015 08:31 AM, Arnd Bergmann wrote:
>>>> On Wednesday 13 May 2015 08:23:41 Brian King wrote:
>>>>> On 05/13/2015 03:10 AM, Arnd Bergmann wrote:
>>>>>> On Tuesday 12 May 2015 18:24:43 Brian King wrote:
>>>>>>>
>>>>>>> Commit 5fb1bf8aaa832e1e9ca3198de7bbecb8eff7db9c broke 64 bit DMA for mpt2sas on Power.
>>>>>>> That commit changed the sequence for setting up the DMA and coherent DMA masks so
>>>>>>> that during initialization the driver requests a 64 bit DMA mask and a 32 bit consistent
>>>>>>> DMA mask, then later requests a 64 bit consistent DMA mask. The Power architecture does
>>>>>>> not currently support this, which results in always falling back to a 32 bit DMA window,
>>>>>>> which has a negative impact on performance. Tweak this algorithm slightly so that
>>>>>>> if requesting a 32 bit consistent mask fails after we've successfully set a 64 bit
>>>>>>> DMA mask, just try to get a 64 bit consistent mask. This should preserve existing
>>>>>>> behavior on platforms that support mixed mask setting and restore previous functionality
>>>>>>> to those that do not.
>>>>>>>
>>>>>>> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
>>>>>>
>>>>>> I believe the way the API is designed, it should guarantee that after dma_set_mask()
>>>>>> succeeds for a device, dma_set_coherent_mask() with the same mask will also succeed.
>>>>>>
>>>>>> Could you just call dma_set_mask_and_coherent() here to avoid that complex logic?
>>>>>
>>>>> I don't think that would work. The mpt2sas driver wants to set the dma mask to 64bits
>>>>> but set the coherent mask to 32 bits, then my change is to set the coherent mask to
>>>>> 64bits if setting it to 32bit fails. I'm not seeing how using dma_set_mask_and_coherent
>>>>> would simplify anything here.
>>>>
>>>> My question was more about why the driver would ask for a 32-bit coherent
>>>> mask to start with. Is this a limitation in the mpt2sas device that can
>>>> only have descriptors at low addresses, or is it trying to work around some
>>>> bug in a particular host system?
>>>
>>> I'll need help from Sreekanth in answering that. All that is in the git commit log is:
>>>
>>>     2. Initially set the consistent DMA mask to 32 bit and then change it to 64 bit mask
>>>     after allocating RDPQ pools by calling the function _base_change_consistent_dma_mask.
>>>     This is to ensure that all the upper 32 bits of RDPQ entries's base address to be same.
>>>
>>> So, I'm not sure my patch won't break something with mpt2sas... This commit also changed
>>> how the RDPO pools are allocated, so its possible this won't work. However, I did load
>>> it on Power box and I'm not seeing any major problems so far. I am seeing the following message
>>> get logged on a regular basis, not sure if its related to this change or not:
>>>
>>> mpt2sas3: log_info(0x31120303): originator(IOP), code(0x03), sub_code(0x3112)
>>>
>>> -Brian
>>>
>>>
>>> --
>>> Brian King
>>> Power Linux I/O
>>> IBM Linux Technology Center
>>>
>>>
> 
> 


-- 
Brian King
Power Linux I/O
IBM Linux Technology Center