linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Will Deacon <Will.Deacon@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Ding Tianhong <dingtianhong@huawei.com>
Subject: Re: For the problem when using swiotlb
Date: Fri, 21 Nov 2014 16:57:09 +0000	[thread overview]
Message-ID: <20141121165708.GF19783@e104818-lin.cambridge.arm.com> (raw)
In-Reply-To: <5029442.vhLMp7Ns7F@wuerfel>

On Fri, Nov 21, 2014 at 12:48:09PM +0000, Arnd Bergmann wrote:
> On Friday 21 November 2014 09:35:10 Catalin Marinas wrote:
> > +static inline int dma_set_coherent_mask(struct device *dev, u64 mask)
> > +{
> > +	if (!dma_supported(dev, mask))
> > +		return -EIO;
> > +	if (mask > dev->coherent_dma_mask)
> > +		mask &= of_dma_get_range_mask(dev);
> > +	dev->coherent_dma_mask = mask;
> > +	return 0;
> > +}
> 
> There is an interesting side problem here: the dma mask points to
> coherent_dma_mask for all devices probed from DT, so this breaks
> if we have any driver that sets them to different values. It is a
> preexisting problem them.

Such drivers would have to set both masks separately (or call
dma_set_mask_and_coherent). What we assume though is that dma-ranges
refers to both dma_mask and coherent_dma_mask. I don't think that would
be a problem for ARM systems.

> >  EXPORT_SYMBOL_GPL(of_dma_get_range);
> >  
> > +u64 of_dma_get_range_mask(struct device *dev)
> > +{
> > +	u64 dma_addr, paddr, size;
> > +
> > +	/* no dma mask limiting if no of_node or no dma-ranges property */
> > +	if (!dev->of_node ||
> > +	    of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size) < 0)
> > +		return DMA_BIT_MASK(64);
> 
> If no dma-ranges are present, we should assume that the bus only supports
> 32-bit DMA, or we could make it architecture specific. It would probably
> be best for arm64 to require a dma-ranges property for doing any DMA
> at all, but we can't do that on arm32 any more now.

I thought about this but it could break some existing arm64 DT files if
we mandate dma-ranges property (we could try though). The mask limiting
is arch-specific anyway.

> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index 3b64d0bf5bba..50d1ac4739e6 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -200,6 +200,10 @@ static void of_dma_configure(struct device *dev)
> >  	/* DMA ranges found. Calculate and set dma_pfn_offset */
> >  	dev->dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
> >  	dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", dev->dma_pfn_offset);
> > +
> > +	/* limit the coherent_dma_mask to the dma-ranges size property */
> 
> I would change the comment to clarify that we are actually changing
> the dma_mask here as well.
> 
> > +	if (size < (1ULL << 32))
> > +		dev->coherent_dma_mask = DMA_BIT_MASK(ilog2(size));
> >  }
> 
> As you mentioned in another mail in this thread, we wouldn't be
> able to suuport this case on arm64.

The mask would still be valid and even usable if an IOMMU is present. Do
you mean we should not limit it at all here?

There is a scenario where smaller mask would work on arm64. For example
Juno, you can have 2GB of RAM in the 32-bit phys range (starting at
0x80000000). A device with 31-bit mask and a dma_pfn_offset of
0x80000000 would still work (there isn't any but just as an example). So
the check in dma_alloc_coherent() would be something like:

	phys_to_dma(top of ZONE_DMA) - dma_pfn_offset <= coherent_dma_mask

(or assuming RAM starts at 0 and ignoring dma_pfn_offset for now)

If the condition above fails, dma_alloc_coherent() would no longer fall
back to swiotlb but issue a dev_warn() and return NULL.

-- 
Catalin

  reply	other threads:[~2014-11-21 16:57 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-17 11:56 For the problem when using swiotlb Ding Tianhong
2014-11-17 12:18 ` Arnd Bergmann
2014-11-17 18:09   ` Catalin Marinas
2014-11-19  3:17     ` Ding Tianhong
2014-11-19  8:45       ` Arnd Bergmann
2014-11-19 11:29         ` Catalin Marinas
2014-11-19 12:48           ` Arnd Bergmann
2014-11-19 15:46             ` Catalin Marinas
2014-11-19 15:56               ` Arnd Bergmann
2014-11-21 11:06                 ` Catalin Marinas
2014-11-21 11:26                   ` Arnd Bergmann
2014-11-21 11:36                     ` Catalin Marinas
2014-11-21 12:27                       ` Arnd Bergmann
2014-11-20  2:57         ` Ding Tianhong
2014-11-20  7:40           ` Arnd Bergmann
2014-11-20  8:34             ` Ding Tianhong
2014-11-20  9:02               ` Arnd Bergmann
2014-11-20  9:21                 ` Ding Tianhong
2014-11-21  9:35             ` Catalin Marinas
2014-11-21 10:32               ` Catalin Marinas
2014-11-21 12:48               ` Arnd Bergmann
2014-11-21 16:57                 ` Catalin Marinas [this message]
2014-11-21 17:04                   ` Arnd Bergmann
2014-11-21 17:51                     ` Catalin Marinas
2014-11-21 18:09                       ` Catalin Marinas
2014-11-24 20:12                         ` Arnd Bergmann
2014-11-25 10:58                           ` Catalin Marinas
2014-11-25 11:29                             ` Russell King - ARM Linux
2014-11-25 12:23                               ` Catalin Marinas
2014-11-27  2:36                                 ` Ding Tianhong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141121165708.GF19783@e104818-lin.cambridge.arm.com \
    --to=catalin.marinas@arm.com \
    --cc=Will.Deacon@arm.com \
    --cc=arnd@arndb.de \
    --cc=dingtianhong@huawei.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).