Re: [PATCH v3 08/16] gpu: host1x: Use correct semantics for HOST1X_CHANNEL_DMAEND

Thierry Reding
2019-02-01T13:41:34Z

[-- Attachment #1.1: Type: text/plain, Size: 2302 bytes --]

On Fri, Feb 01, 2019 at 04:37:59PM +0300, Dmitry Osipenko wrote:
> 01.02.2019 16:28, Thierry Reding пишет:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > The HOST1X_CHANNEL_DMAEND is an offset relative to the value written to
> > the HOST1X_CHANNEL_DMASTART register, but it is currently treated as an
> > absolute address. This can cause SMMU faults if the CDMA fetches past a
> > pushbuffer's IOMMU mapping.
> >
> > Properly setting the DMAEND prevents the CDMA from fetching beyond that
> > address and avoid such issues. This is currently not observed because a
> > whole (almost) page of essentially scratch space absorbs any excessive
> > prefetching by CDMA. However, changing the number of slots in the push
> > buffer can trigger these SMMU faults.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > drivers/gpu/host1x/hw/cdma_hw.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/host1x/hw/cdma_hw.c b/drivers/gpu/host1x/hw/cdma_hw.c
> > index 485aef5761af..a24c090ac96f 100644
> > --- a/drivers/gpu/host1x/hw/cdma_hw.c
> > +++ b/drivers/gpu/host1x/hw/cdma_hw.c
> > @@ -75,7 +75,7 @@ static void cdma_start(struct host1x_cdma *cdma)
> >
> > cdma->last_pos = cdma->push_buffer.pos;
> > start = cdma->push_buffer.dma;
> > - end = start + cdma->push_buffer.size + 4;
> > + end = cdma->push_buffer.size + 4;
> >
> > host1x_ch_writel(ch, HOST1X_CHANNEL_DMACTRL_DMASTOP,
> > HOST1X_CHANNEL_DMACTRL);
> > @@ -126,7 +126,7 @@ static void cdma_timeout_restart(struct host1x_cdma *cdma, u32 getptr)
> > HOST1X_CHANNEL_DMACTRL);
> >
> > start = cdma->push_buffer.dma;
> > - end = start + cdma->push_buffer.size + 4;
> > + end = cdma->push_buffer.size + 4;
> >
> > /* set base, end pointer (all of memory) */
> > host1x_ch_writel(ch, lower_32_bits(start), HOST1X_CHANNEL_DMASTART);
> >
>
> This seems fixes problem that was added by a previous patch in this
> series, "gpu: host1x: Support 40-bit addressing". What about to just
> squash the patches?

This actually fixes a bug that's always been there. This just happens to
touch the same lines as an earlier patch as a result of some refactoring
that the earlier patch did.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]