RE: [PATCH 1/3] block: Respect the device's maximum segment size

From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: Thierry Reding <thierry.reding@gmail.com>,
	Christoph Hellwig <hch@lst.de>,
	Ulf Hansson <ulf.hansson@linaro.org>
Cc: Jens Axboe <axboe@kernel.dk>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Simon Horman <horms+renesas@verge.net.au>,
	Jon Hunter <jonathanh@nvidia.com>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>
Subject: RE: [PATCH 1/3] block: Respect the device's maximum segment size
Date: Tue, 10 Sep 2019 02:03:17 +0000	[thread overview]
Message-ID: <TYAPR01MB454470364B682B9BF708E557D8B60@TYAPR01MB4544.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <20190909191911.GC23804@mithrandir>

Hi Thierry,

> From: Thierry Reding, Sent: Tuesday, September 10, 2019 4:19 AM
> 
> On Mon, Sep 09, 2019 at 06:13:31PM +0200, Christoph Hellwig wrote:
> > On Mon, Sep 09, 2019 at 02:56:56PM +0200, Thierry Reding wrote:
> > > From: Thierry Reding <treding@nvidia.com>
> > >
> > > When enabling the DMA map merging capability for a queue, ensure that
> > > the maximum segment size does not exceed the device's limit.
> >
> > We can't do that unfortunately.  If we use the virt_boundary setting
> > we do aggressive merges that there is no accounting for.  So we can't
> > limit the segment size.
> 
> But that's kind of the point here. My understanding is that the maximum
> segment size in the device's DMA parameters defines the maximum size of
> the segment that the device can handle.
> 
> In the particular case that I'm trying to fix, according to the SDHCI
> specification, these devices can handle segments that are a maximum of
> 64 KiB in size. If we allow that segment size to be exceeded, the device
> will no longer be able to handle them.
> 
> > And at least for the case how devices usually do the addressing
> > (page based on not sgl based) that needs the virt_boundary there isn't
> > really any concept like a segment anyway.
> 
> I do understand that aspect of it. However, devices that do the
> addressing this way, wouldn't they want to set the maximum segment size
> to something large (like UINT_MAX, which many users that don't have the
> concept of a segment already do)?
> 
> If you disagree, do you have any alternative proposals other than
> reverting the offending patch? linux-next is currently broken on all
> systems where the SDHCI controller is behind an IOMMU.

I'm sorry for causing this trouble on your environment. I have a proposal to
resolve this issue. The mmc_host struct will have a new caps2 flag
like MMC_CAP2_MERGE_CAPABLE and add a condition into the queue.c like below.

+	if (host->caps2 & MMC_CAP2_MERGE_CAPABLE &&
+	    host->max_segs < MMC_DMA_MAP_MERGE_SEGMENTS &&
	    dma_get_merge_boundary(mmc_dev(host)))
		host->can_dma_map_merge = 1;
	else
		host->can_dma_map_merge = 0;

After that, all mmc controllers disable the feature as default, and if a mmc
controller has such capable, the host driver should set the flag.
Ulf, is such a patch acceptable for v5.4-rcN? Anyway, I'll submit such a patch later.

Best regards,
Yoshihiro Shimoda

> Thierry