Re: [PATCH 4/4] mmc: sdhci: Add DMA memory boundary workaround

From: Ulf Hansson <ulf.hansson@linaro.org>
To: Jisheng Zhang <Jisheng.Zhang@synaptics.com>,
	Christoph Hellwig <hch@infradead.org>
Cc: Jun Nie <jun.nie@linaro.org>, Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	"adrian.hunter@intel.com" <adrian.hunter@intel.com>,
	linux-mmc <linux-mmc@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH 4/4] mmc: sdhci: Add DMA memory boundary workaround
Date: Tue, 10 Dec 2019 10:36:49 +0100	[thread overview]
Message-ID: <CAPDyKFrreSgNfHX9hSKvYO7UxBEyqu0DSxX9NGp4phbR-F1fxQ@mail.gmail.com> (raw)
In-Reply-To: <20191204150043.3a82cec0@xhacker.debian>

On Wed, 4 Dec 2019 at 08:14, Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:
>
> On Wed, 4 Dec 2019 14:00:08 +0800 Jun Nie wrote:
>
> >
> >
> > Christoph Hellwig <hch@infradead.org> 于2019年12月3日周二 下午3:36写道：
> > >
> > > On Tue, Dec 03, 2019 at 11:29:15AM +0800, Jun Nie wrote:
> > > > Thanks for the reminder! So I need to parse the segment_boundary from
> > > > device tree and use below code to set it, right?
> > > > For the max_segments accounting error, I did not see it so far though I
> > > > believe it is true in theory. Maybe it is due to segment boundary value is
> > > > very large.
> > > >
> > > > +++ b/drivers/mmc/core/queue.c
> > > > @@ -374,6 +374,7 @@ static void mmc_setup_queue(struct mmc_queue *mq,
> > > > struct mmc_card *card)
> > > >                 WARN(!blk_queue_can_use_dma_map_merging(mq->queue,
> > > >                                                         mmc_dev(host)),
> > > >                      "merging was advertised but not possible");
> > > > +       blk_queue_segment_boundary(mq->queue, mmc->segment_boundary);
> > > >         blk_queue_max_segments(mq->queue, mmc_get_max_segments(host));
> > >
> > > Yes, I think should do it.  Maybe modulo a check if the low-level
> > > driver actually sets a segment boundary.
> >
> > For the block device, such as SD card, it is right solution. But I
> > have concern on SDIO case. Maybe we should add workaround together
> > with block layer segment boundary restriction. How do you think about
> > it?
> >

Yes, buffers for SDIO are a consern. Especially since those buffers
are allocated by SDIO func drivers or even from upper layers, such as
the network stacks, for example.

I think SDIO func drivers simply need to respect the constraints the
host has set via the .segment_boundary, .max_seg_size, .max_segs, etc.
We should export SDIO func APIs to make the information available for
the SDIO func drivers.

>
> Another trouble is how to workaround if the sg is constructed by mmc and
> no block layer interactions at all. e.g __mmc_blk_ioctl_cmd(), and all
> those sgs in mmc_test.c

Those should be easier to fix, as the buffer/sg allocation can be
fixed internally by mmc core. Just post some patches. :-)

I am more worried about SDIO, as those buffers are not that easy to control.

Note that, there have been suggestions on adding an SDIO interface
where an sg can be passed [1]. Unfurtunate those patches got stuck and
didn't make it.

Kind regards
Uffe

[1]
https://patchwork.kernel.org/patch/10123143/