All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Nikita Yushchenko <nikita.yoush@cogentembedded.com>,
	Will Deacon <will.deacon@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	Simon Horman <horms@verge.net.au>,
	Bjorn Helgaas <bhelgaas@google.com>,
	artemi.ivanov@cogentembedded.com, fkan@apm.com,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle
Date: Tue, 10 Jan 2017 16:06:29 +0100	[thread overview]
Message-ID: <1900630.ldF24KWUdV@wuerfel> (raw)
In-Reply-To: <5c5cd4fd-4854-a2dd-10b6-9cc98e63a85c@arm.com>

On Tuesday, January 10, 2017 2:16:57 PM CET Robin Murphy wrote:
> On 10/01/17 13:42, Arnd Bergmann wrote:
> > On Tuesday, January 10, 2017 1:25:12 PM CET Robin Murphy wrote:
> >> On 10/01/17 12:47, Nikita Yushchenko wrote:
> >>>> The point here is that an IOMMU doesn't solve your issue, and the
> >>>> IOMMU-backed DMA ops need the same treatment. In light of that, it really
> >>>> feels to me like the DMA masks should be restricted in of_dma_configure
> >>>> so that the parent mask is taken into account there, rather than hook
> >>>> into each set of DMA ops to intercept set_dma_mask. We'd still need to
> >>>> do something to stop dma_set_mask widening the mask if it was restricted
> >>>> by of_dma_configure, but I think Robin (cc'd) was playing with that.
> >>>
> >>> What issue "IOMMU doesn't solve"?
> >>>
> >>> Issue I'm trying to address is - inconsistency within swiotlb
> >>> dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
> >>> mask is used to decide if bounce buffers are needed or not. This
> >>> inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
> >>> instead).
> >>
> >> The fundamental underlying problem is the "any wide mask is silently
> >> accepted" part, and that applies equally to IOMMU ops as well.
> > 
> > It's a much rarer problem for the IOMMU case though, because it only
> > impacts devices that are restricted to addressing of less than 32-bits.
> > 
> > If you have an IOMMU enabled, the dma-mapping interface does not care
> > if the device can do wider than 32 bit addressing, as it will never
> > hand out IOVAs above 0xffffffff.
> 
> I can assure you that it will - we constrain allocations to the
> intersection of the IOMMU domain aperture (normally the IOMMU's physical
> input address width) and the given device's DMA mask. If both of those
> are >32 bits then >32-bit IOVAs will fall out. For the arm64/common
> implementation I have prototyped a copy of the x86 optimisation which
> always first tries to get 32-bit IOVAs for PCI devices, but even then it
> can start returning higher addresses if the 32-bit space fills up.

Ok, got it. I have to admit that most of my knowledge about the internals
of IOMMUs is from PowerPC of a few years ago, which couldn't do this at
all. I agree that we need to do the same thing on swiotlb and iommu then.

> >> The thread Will linked to describes that equivalent version of your
> >> problem - the IOMMU gives the device 48-bit addresses which get
> >> erroneously truncated because it doesn't know that only 42 bits are
> >> actually wired up. That situation still requires the device's DMA mask
> >> to correctly describe its addressing capability just as yours does.
> > 
> > That problem should only impact virtual machines which have a guest
> > bus address space covering more than 42 bits of physical RAM, whereas
> > the problem we have with swiotlb is for the dma-mapping interface.
> > 
> I actually have a third variation of this problem involving a PCI root
> complex which *could* drive full-width (40-bit) addresses, but won't,
> due to the way its PCI<->AXI interface is programmed. That would require
> even more complicated dma-ranges handling to describe the windows of
> valid physical addresses which it *will* pass, so I'm not pressing the
> issue - let's just get the basic DMA mask case fixed first.

Can you describe this a little more? We should at least try to not
make it harder to solve the next problem while solving this one,
so I'd like to understand the exact limitation you are hitting there.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] arm64: do not set dma masks that device connection can't handle
Date: Tue, 10 Jan 2017 16:06:29 +0100	[thread overview]
Message-ID: <1900630.ldF24KWUdV@wuerfel> (raw)
In-Reply-To: <5c5cd4fd-4854-a2dd-10b6-9cc98e63a85c@arm.com>

On Tuesday, January 10, 2017 2:16:57 PM CET Robin Murphy wrote:
> On 10/01/17 13:42, Arnd Bergmann wrote:
> > On Tuesday, January 10, 2017 1:25:12 PM CET Robin Murphy wrote:
> >> On 10/01/17 12:47, Nikita Yushchenko wrote:
> >>>> The point here is that an IOMMU doesn't solve your issue, and the
> >>>> IOMMU-backed DMA ops need the same treatment. In light of that, it really
> >>>> feels to me like the DMA masks should be restricted in of_dma_configure
> >>>> so that the parent mask is taken into account there, rather than hook
> >>>> into each set of DMA ops to intercept set_dma_mask. We'd still need to
> >>>> do something to stop dma_set_mask widening the mask if it was restricted
> >>>> by of_dma_configure, but I think Robin (cc'd) was playing with that.
> >>>
> >>> What issue "IOMMU doesn't solve"?
> >>>
> >>> Issue I'm trying to address is - inconsistency within swiotlb
> >>> dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
> >>> mask is used to decide if bounce buffers are needed or not. This
> >>> inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
> >>> instead).
> >>
> >> The fundamental underlying problem is the "any wide mask is silently
> >> accepted" part, and that applies equally to IOMMU ops as well.
> > 
> > It's a much rarer problem for the IOMMU case though, because it only
> > impacts devices that are restricted to addressing of less than 32-bits.
> > 
> > If you have an IOMMU enabled, the dma-mapping interface does not care
> > if the device can do wider than 32 bit addressing, as it will never
> > hand out IOVAs above 0xffffffff.
> 
> I can assure you that it will - we constrain allocations to the
> intersection of the IOMMU domain aperture (normally the IOMMU's physical
> input address width) and the given device's DMA mask. If both of those
> are >32 bits then >32-bit IOVAs will fall out. For the arm64/common
> implementation I have prototyped a copy of the x86 optimisation which
> always first tries to get 32-bit IOVAs for PCI devices, but even then it
> can start returning higher addresses if the 32-bit space fills up.

Ok, got it. I have to admit that most of my knowledge about the internals
of IOMMUs is from PowerPC of a few years ago, which couldn't do this at
all. I agree that we need to do the same thing on swiotlb and iommu then.

> >> The thread Will linked to describes that equivalent version of your
> >> problem - the IOMMU gives the device 48-bit addresses which get
> >> erroneously truncated because it doesn't know that only 42 bits are
> >> actually wired up. That situation still requires the device's DMA mask
> >> to correctly describe its addressing capability just as yours does.
> > 
> > That problem should only impact virtual machines which have a guest
> > bus address space covering more than 42 bits of physical RAM, whereas
> > the problem we have with swiotlb is for the dma-mapping interface.
> > 
> I actually have a third variation of this problem involving a PCI root
> complex which *could* drive full-width (40-bit) addresses, but won't,
> due to the way its PCI<->AXI interface is programmed. That would require
> even more complicated dma-ranges handling to describe the windows of
> valid physical addresses which it *will* pass, so I'm not pressing the
> issue - let's just get the basic DMA mask case fixed first.

Can you describe this a little more? We should at least try to not
make it harder to solve the next problem while solving this one,
so I'd like to understand the exact limitation you are hitting there.

	Arnd

  reply	other threads:[~2017-01-10 15:07 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-09  7:30 [PATCH v2] arm64: do not set dma masks that device connection can't handle Nikita Yushchenko
2017-01-09  7:30 ` Nikita Yushchenko
2017-01-10 11:51 ` Will Deacon
2017-01-10 11:51   ` Will Deacon
2017-01-10 12:47   ` Nikita Yushchenko
2017-01-10 12:47     ` Nikita Yushchenko
2017-01-10 13:12     ` Arnd Bergmann
2017-01-10 13:12       ` Arnd Bergmann
2017-01-10 13:25     ` Robin Murphy
2017-01-10 13:25       ` Robin Murphy
2017-01-10 13:42       ` Arnd Bergmann
2017-01-10 13:42         ` Arnd Bergmann
2017-01-10 14:16         ` Robin Murphy
2017-01-10 14:16           ` Robin Murphy
2017-01-10 15:06           ` Arnd Bergmann [this message]
2017-01-10 15:06             ` Arnd Bergmann
2017-01-11 12:37           ` Nikita Yushchenko
2017-01-11 12:37             ` Nikita Yushchenko
2017-01-11 16:21             ` Arnd Bergmann
2017-01-11 16:21               ` Arnd Bergmann
2017-01-11 18:28             ` Robin Murphy
2017-01-11 18:28               ` Robin Murphy
2017-01-10 14:59         ` Christoph Hellwig
2017-01-10 14:59           ` Christoph Hellwig
2017-01-10 14:00       ` [PATCH] arm64: avoid increasing DMA masks above what hardware supports Nikita Yushchenko
2017-01-10 14:00         ` Nikita Yushchenko
2017-01-10 17:14         ` Robin Murphy
2017-01-10 17:14           ` Robin Murphy
2017-01-11  7:59           ` Nikita Yushchenko
2017-01-11  7:59             ` Nikita Yushchenko
2017-01-11 11:54             ` Robin Murphy
2017-01-11 11:54               ` Robin Murphy
2017-01-11 13:41               ` Nikita Yushchenko
2017-01-11 13:41                 ` Nikita Yushchenko
2017-01-11 14:50                 ` Robin Murphy
2017-01-11 14:50                   ` Robin Murphy
2017-01-11 16:03                   ` Nikita Yushchenko
2017-01-11 16:50                     ` Robin Murphy
2017-01-11 16:50                       ` Robin Murphy
2017-01-11 18:31           ` [PATCH 0/2] arm64: fix handling of DMA masks wider than bus supports Nikita Yushchenko
2017-01-11 18:31             ` Nikita Yushchenko
2017-01-11 18:31             ` [PATCH 1/2] dma-mapping: let arch know origin of dma range passed to arch_setup_dma_ops() Nikita Yushchenko
2017-01-11 18:31               ` Nikita Yushchenko
2017-01-11 21:08               ` Arnd Bergmann
2017-01-11 21:08                 ` Arnd Bergmann
2017-01-12  5:52                 ` Nikita Yushchenko
2017-01-12  5:52                   ` Nikita Yushchenko
2017-01-12  6:33                   ` Nikita Yushchenko
2017-01-12  6:33                     ` Nikita Yushchenko
2017-01-12 13:28                     ` Arnd Bergmann
2017-01-12 13:28                       ` Arnd Bergmann
2017-01-12 13:39                       ` Nikita Yushchenko
2017-01-12 13:39                         ` Nikita Yushchenko
2017-01-12 12:16                   ` Will Deacon
2017-01-12 12:16                     ` Will Deacon
2017-01-12 13:25                     ` Arnd Bergmann
2017-01-12 13:25                       ` Arnd Bergmann
2017-01-12 13:43                       ` Robin Murphy
2017-01-12 13:43                         ` Robin Murphy
2017-01-13 10:40               ` kbuild test robot
2017-01-13 10:40                 ` kbuild test robot
2017-01-11 18:31             ` [PATCH 2/2] arm64: avoid increasing DMA masks above what hardware supports Nikita Yushchenko
2017-01-11 18:31               ` Nikita Yushchenko
2017-01-11 21:11               ` Arnd Bergmann
2017-01-11 21:11                 ` Arnd Bergmann
2017-01-12  5:53                 ` Nikita Yushchenko
2017-01-12  5:53                   ` Nikita Yushchenko
2017-01-13 10:16               ` kbuild test robot
2017-01-13 10:16                 ` kbuild test robot
2017-01-10 14:01       ` [PATCH v2] arm64: do not set dma masks that device connection can't handle Nikita Yushchenko
2017-01-10 14:01         ` Nikita Yushchenko
2017-01-10 14:57       ` Christoph Hellwig
2017-01-10 14:57         ` Christoph Hellwig
2017-01-10 14:51     ` Christoph Hellwig
2017-01-10 14:51       ` Christoph Hellwig

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=1900630.ldF24KWUdV@wuerfel \
    --to=arnd@arndb.de \
    --cc=artemi.ivanov@cogentembedded.com \
    --cc=bhelgaas@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=fkan@apm.com \
    --cc=hch@lst.de \
    --cc=horms@verge.net.au \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=nikita.yoush@cogentembedded.com \
    --cc=robin.murphy@arm.com \
    --cc=will.deacon@arm.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.