On Wed, Sep 23, 2020 at 03:34:09PM -0300, Jason Gunthorpe wrote:
> device == struct ib_device we just allocated
>
> The only use of this configuration is to override the dma ops with
> dma_virt_ops, so drivers that don't do that are buggy. A ib_device
> itself cannot do DMA otherwise. This should probably be clarified to
> just fail if !CONIFG_DMA_OPS
>
> All other cases should point dma_device at some kind of DMA capable
> struct device like a pci_device, which can have a NULL ops.
This makes some sense. Of course this is really weird by keeping
the virt_ops assignment in the driver while this logic is here, creating
a bit of a mess.
> > > + } else {
> > > + device->dev.dma_parms = dma_device->dma_parms;
> > > /*
> > > + * Auto setup the segment size if a DMA device was passed in.
> > > + * The PCI core sets the maximum segment size to 64 KB. Increase
> > > + * this parameter to 2 GB.
> > > */
> > > + dma_set_max_seg_size(dma_device, SZ_2G);
> >
> > You can't just inherity DMA properties like this this. Please
> > fix all code that looks at the seg size to look at the DMA device.
>
> Inherit? This is overriding the PCI default of 64K to be 2G for RDMA
> devices.
With inherit I mean the
device->dev.dma_parms = dma_device->dma_parms;
line, which is completely bogus. All DMA mapping is done on the
dma_device in the RDMA core and ULPs, so it also can't have an effect.
> The closest thing RDMA has to segment size is the length of a IB
> scatter/gather WR element in verbs. This is 32 bits by spec.
>
> Even if a SGL > 32 bits was required the ULP should switch to use RDMA
> MRs instead of inline IB SG.
>
> So really there is no segment size limitation and the intention here
> is to just disable segment size at IOMMU layer.
>
> Since this is universal, by spec, not HW specific, it doesn't make
> much sense to put in the drivers.
What if your DMA device is shared by non-RDMA functionality such
as a network or storage device which would like an even larger limit?
