Re: pci-usb/pci-sata broken with LPAE config after "reduce use of block bounce buffers"

[Kishon Vijay Abraham I <kishon@ti.com>]

Hi Christoph,

On 2/18/2020 5:45 PM, Kishon Vijay Abraham I wrote:
> Christoph,
> 
> On 17/02/20 7:53 pm, Christoph Hellwig wrote:
>> On Wed, Feb 05, 2020 at 05:05:42PM +0100, Christoph Hellwig wrote:
>>> On Wed, Feb 05, 2020 at 03:03:13PM +0530, Kishon Vijay Abraham I wrote:
>>>> Yes, I see the mismatch after reverting the above patches.
>>>
>>> In which case the data mismatch is very likely due to a different root
>>> cause.
>>
>> Did you manage to dig into this a little more?
> 
> I'll probably get to this later half of this week. Will update you then.
> 

Sorry for the delay in getting back to this. But I guess I have root caused the
issue now.

The issue was because NVMe is requesting a sector size (4096KB) which is more
than what is supported by SWIOTLB default (256KB). NVMe driver actually has a
mechanism to select the correct sector size

 dev->ctrl.max_hw_sectors = min_t(u32,
                NVME_MAX_KB_SZ << 1, dma_max_mapping_size(dev->dev) >> 9);
However dma_max_mapping_size() here misbehaves and gives 4G. Ideally it should
have given 256KB -> the max supported by SWIOTLB

Tracing through the dma_max_mapping_size(), dma_direct_max_mapping_size() was
giving incorrect value

size_t dma_direct_max_mapping_size(struct device *dev)
{
        /* If SWIOTLB is active, use its maximum mapping size */
        if (is_swiotlb_active() &&
            (dma_addressing_limited(dev) || swiotlb_force == SWIOTLB_FORCE))
                return swiotlb_max_mapping_size(dev);
        return SIZE_MAX;
}
In the above function swiotlb_max_mapping_size(dev) gives 256KB however
dma_addressing_limited(dev) always returns false. So 256KB is never returned to
the NVMe driver.

Tracing dma_addressing_limited(dev), found a bug in
dma_direct_get_required_mask(). When it passes the physical address to
phys_to_dma_direct(), the upper 32 bit is lost and dma_addressing_limited(dev)
thinks the entire address is accessible by the device.

A patch that type casts the argument of phys_to_dma_direct() like below fixes
the issue.

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 32ec69cdba54..0081410334c8 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -51,7 +51,9 @@ static inline struct page *dma_direct_to_page(struct device
*dev, u64 dma_direct_get_required_mask(struct device *dev)
 {
-       u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
+       u64 max_dma =
+               phys_to_dma_direct(dev,
+                                  (phys_addr_t)(max_pfn - 1) << PAGE_SHIFT);
        return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
 }

If this looks okay to you, I can post a patch for it.

Thanks
Kishon