Re: [PATCH v2 02/15] NTB: Set dma mask and dma coherent mask to NTB devices

From: Logan Gunthorpe
Date: Tue Dec 05 2017 - 12:22:07 EST




On 05/12/17 09:51 AM, Jon Mason wrote:
On Sun, Dec 3, 2017 at 2:17 PM, Serge Semin <fancer.lancer@xxxxxxxxx> wrote:
The dma_mask and dma_coherent_mask fields of the NTB struct device
weren't initialized in hardware drivers. In fact it should be done
instead of PCIe interface usage, since NTB clients are supposed to
use NTB API only and left unaware of real hardware implementation.
In addition to that ntb_device_register() method shouldn't clear
the passed ntb_dev structure, since it dma_mask is initialized
by hardware drivers.

This seems like a bug fix. Should this be broken out of this patch or
is it not noticed without your changes?

I was digging into this a bit more and in addition to my earlier review comments I think the patch is incorrect for a couple reasons:

1) dma_set_mask() (and friends) are meant to inform the kernel of device specific restrictions it cannot know. Calling it with the parent PCI device's mask seems pointless.

2) Every dma_alloc() call in the NTB subsystem uses the PCI device's struct device. This is ugly in itself but until it changes this patch does nothing.

Logan