Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
From: Andrew Murray <andrew.murray@arm.com>
To: Marek Vasut <marek.vasut@gmail.com>
Cc: Simon Horman <horms@verge.net.au>,
	linux-pci@vger.kernel.org,
	Marek Vasut <marek.vasut+renesas@gmail.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Wolfram Sang <wsa@the-dreams.de>,
	linux-renesas-soc@vger.kernel.org, Robin.Murphy@arm.com
Subject: Re: [PATCH V3 2/3] PCI: rcar: Do not abort on too many inbound dma-ranges
Date: Sat, 26 Oct 2019 21:36:28 +0100
Message-ID: <20191026203627.GA47056@e119886-lin.cambridge.arm.com> (raw)
In-Reply-To: <fef9502f-d51c-b922-afb3-8891267ae6c3@gmail.com>

On Sat, Oct 26, 2019 at 08:03:12PM +0200, Marek Vasut wrote:
> On 10/21/19 12:18 PM, Andrew Murray wrote:
> [...]
> >>>> In case the "dma-ranges" DT property contains either too many ranges
> >>>> or the range start address is unaligned in such a way that populating
> >>>> the range into the controller requires multiple entries, a situation
> >>>> may occur where all ranges cannot be loaded into the controller.
> >>>>
> >>>> Currently, the driver refuses to probe in such a situation. Relax this
> >>>> behavior, load as many ranges as possible and warn if some ranges do
> >>>> not fit anymore.
> >>>
> >>> What is the motivation for relaxing this?
> >>
> >> U-Boot can fill the ranges in properly now, the list would be longer in
> >> such a case and the driver would fail to probe (because the list is
> >> longer than what the hardware can support).
> > 
> > Is this the U-Boot patch you refer to:
> > 
> > https://patchwork.ozlabs.org/patch/1129436/
> 
> Yes.
> 
> > As pci_set_region is called with the same address for PCI and CPU memory
> > this implies there is a 1:1 mapping - therefore I don't see a need for
> > multiple mappings for each DRAM bank. (Also if this controller has a
> > 32 bit limitation, shouldn't this code limit the addresses before calling
> > pci_set_region?).
> It would certainly be helpful to know about this dma-ranges detail
> earlier, this whole thing could've been avoided. Now all I can do is get
> that patch reverted for the next U-Boot release.

Yes, I can appreciate the frustration this delay has caused. Though as there
are now more reviewers for PCI controllers on this list, future patches ought
to get feedback sooner.

> 
> But this still leaves me with one open question -- how do I figure out
> what to program into the PCI controller inbound windows, so that the
> controller correctly filters inbound transfers which are targetting
> nonexisting memory ?

Your driver should program into the RC->CPU windows, the exact ranges
described in the dma-ranges. Whilst also respecting the alignment and
max-size rules your controller has (e.g. the existing upstream logic
and also the new logic that recalculates the alignment per entry).

As far as I can tell from looking at your U-Boot patch, I think I'd expect
a single dma-range to be presented in the DT, that describes
0:0xFFFFFFFF => 0:0xFFFFFFFF. This is because 1) I understand your
controller is limited to 32 bits. And 2) there is a linear mapping between
PCI and CPU addresses (given that the second and third arguments on
pci_set_region are both the same).

As you point out, this range includes lots of things that you don't
want the RC to touch - such as non-existent memory. This is OK, when
Linux programs addresses into the various EP's for them to DMA to host
memory, it uses its own logic to select addresses that are in RAM, the
purpose of the dma-range is to describe what the CPU RAM address looks
like from the perspective of the RC (for example if the RC was wired
with an offset such that made memory writes from the RC made to
0x00000000 end up on the system map at 0x80000000, we need to tell Linux
about this offset. Otherwise when a EP device driver programs a DMA
address of a RAM buffer at 0x90000000, it'll end up targetting
0x110000000. Thankfully our dma-range will tell Linux to apply an offset
such that the actual address written to the EP is 0x10000000.).

In your case the dma-range also serves to describe a limit to the range
of addresses we can reach.

Thanks,

Andrew Murray

> 
> -- 
> Best regards,
> Marek Vasut

  reply index

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-09 17:57 [PATCH V3 1/3] PCI: rcar: Move the inbound index check marek.vasut
2019-08-09 17:57 ` [PATCH V3 2/3] PCI: rcar: Do not abort on too many inbound dma-ranges marek.vasut
2019-08-16 13:23   ` Simon Horman
2019-08-16 13:28     ` Marek Vasut
2019-08-16 13:38       ` Simon Horman
2019-08-16 17:41         ` Marek Vasut
2019-10-21 10:18       ` Andrew Murray
2019-10-26 18:03         ` Marek Vasut
2019-10-26 20:36           ` Andrew Murray [this message]
2019-10-26 21:06             ` Andrew Murray
2019-11-06 23:37             ` Marek Vasut
2019-11-07 14:19               ` Andrew Murray
2019-11-16 15:48                 ` Marek Vasut
2019-11-18 18:42                   ` Robin Murphy
2019-10-16 15:00   ` Lorenzo Pieralisi
2019-10-16 15:10     ` Marek Vasut
2019-10-16 15:26       ` Lorenzo Pieralisi
2019-10-16 15:29         ` Marek Vasut
2019-10-16 16:18           ` Lorenzo Pieralisi
2019-10-16 18:12             ` Rob Herring
2019-10-16 18:17               ` Marek Vasut
2019-10-16 20:25                 ` Rob Herring
2019-10-16 21:15                   ` Marek Vasut
2019-10-16 22:26                     ` Rob Herring
2019-10-16 22:33                       ` Marek Vasut
2019-10-17  7:06                         ` Geert Uytterhoeven
2019-10-17 10:55                           ` Marek Vasut
2019-10-17 13:06                             ` Robin Murphy
2019-10-17 14:00                               ` Marek Vasut
2019-10-17 14:36                                 ` Rob Herring
2019-10-17 15:01                                   ` Marek Vasut
2019-10-18  9:53                                     ` Lorenzo Pieralisi
2019-10-18 12:22                                       ` Marek Vasut
2019-10-18 12:53                                         ` Robin Murphy
2019-10-18 14:26                                           ` Marek Vasut
2019-10-18 15:44                                             ` Robin Murphy
2019-10-18 16:44                                               ` Marek Vasut
2019-10-18 17:35                                                 ` Robin Murphy
2019-10-18 18:44                                                   ` Marek Vasut
2019-10-21  8:32                                                     ` Geert Uytterhoeven
2019-11-19 12:10                                                     ` Robin Murphy
2019-10-18 10:06                         ` Andrew Murray
2019-10-18 10:17                           ` Geert Uytterhoeven
2019-10-18 11:40                             ` Andrew Murray
2019-08-09 17:57 ` [PATCH V3 3/3] PCI: rcar: Recalculate inbound range alignment for each controller entry marek.vasut
2019-10-21 10:39   ` Andrew Murray
2019-08-16 10:52 ` [PATCH V3 1/3] PCI: rcar: Move the inbound index check Lorenzo Pieralisi
2019-08-16 10:59   ` Marek Vasut
2019-08-16 11:10     ` Lorenzo Pieralisi
2019-10-15 20:14 ` Marek Vasut
2019-10-21 10:11 ` Andrew Murray

Reply instructions:

You may reply publically 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=20191026203627.GA47056@e119886-lin.cambridge.arm.com \
    --to=andrew.murray@arm.com \
    --cc=Robin.Murphy@arm.com \
    --cc=geert+renesas@glider.be \
    --cc=horms@verge.net.au \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=marek.vasut+renesas@gmail.com \
    --cc=marek.vasut@gmail.com \
    --cc=wsa@the-dreams.de \
    /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

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git