Re: [PATCH rdma-next] RDMA: Explicitly pass in the dma_device to ib_register_device

From: Jason Gunthorpe <jgg@nvidia.com>
To: Christoph Hellwig <hch@infradead.org>,
	Bart Van Assche <bvanassche@acm.org>
Cc: Leon Romanovsky <leon@kernel.org>,
	Doug Ledford <dledford@redhat.com>,
	Adit Ranadive <aditr@vmware.com>,
	Ariel Elior <aelior@marvell.com>,
	"Bernard Metzler" <bmt@zurich.ibm.com>,
	Christian Benvenuti <benve@cisco.com>,
	"Dennis Dalessandro" <dennis.dalessandro@intel.com>,
	Devesh Sharma <devesh.sharma@broadcom.com>,
	Faisal Latif <faisal.latif@intel.com>,
	"Gal Pressman" <galpress@amazon.com>,
	Lijun Ou <oulijun@huawei.com>, <linux-rdma@vger.kernel.org>,
	Michal Kalderon <mkalderon@marvell.com>,
	"Mike Marciniszyn" <mike.marciniszyn@intel.com>,
	Naresh Kumar PBS <nareshkumar.pbs@broadcom.com>,
	Nelson Escobar <neescoba@cisco.com>,
	"Parav Pandit" <parav@nvidia.com>,
	Parvi Kaustubhi <pkaustub@cisco.com>,
	"Potnuri Bharat Teja" <bharat@chelsio.com>,
	Selvin Xavier <selvin.xavier@broadcom.com>,
	Shiraz Saleem <shiraz.saleem@intel.com>,
	Somnath Kotur <somnath.kotur@broadcom.com>,
	Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>,
	VMware PV-Drivers <pv-drivers@vmware.com>,
	Weihang Li <liweihang@huawei.com>,
	"Wei Hu(Xavier)" <huwei87@hisilicon.com>,
	Yishai Hadas <yishaih@nvidia.com>,
	Zhu Yanjun <yanjunz@nvidia.com>
Subject: Re: [PATCH rdma-next] RDMA: Explicitly pass in the dma_device to ib_register_device
Date: Thu, 24 Sep 2020 08:49:40 -0300	[thread overview]
Message-ID: <20200924114940.GE9475@nvidia.com> (raw)
In-Reply-To: <20200924054907.GA22045@infradead.org>

On Thu, Sep 24, 2020 at 06:49:07AM +0100, Christoph Hellwig wrote:
> > > > +	} 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.

Oh. Yes, no idea why that is there..

commit c9121262d57b8a3be4f08073546436ba0128ca6a
Author: Bart Van Assche <bvanassche@acm.org>
Date:   Fri Oct 25 15:58:30 2019 -0700

    RDMA/core: Set DMA parameters correctly

    The dma_set_max_seg_size() call in setup_dma_device() does not have any
    effect since device->dev.dma_parms is NULL. Fix this by initializing
    device->dev.dma_parms first.

Bart?

> > 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?

This limit should be the largest possible, if we can go higher here,
then lets go higher. UINT_MAX?

Hopefully nobody needs lower in the multi-function case

Jason