From: Robin Murphy <firstname.lastname@example.org> To: Marek Vasut <email@example.com>, Andrew Murray <firstname.lastname@example.org> Cc: Simon Horman <email@example.com>, firstname.lastname@example.org, Marek Vasut <email@example.com>, Geert Uytterhoeven <firstname.lastname@example.org>, Lorenzo Pieralisi <email@example.com>, Wolfram Sang <firstname.lastname@example.org>, email@example.com Subject: Re: [PATCH V3 2/3] PCI: rcar: Do not abort on too many inbound dma-ranges Date: Mon, 18 Nov 2019 18:42:53 +0000 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> On 16/11/2019 3:48 pm, Marek Vasut wrote: > On 11/7/19 3:19 PM, Andrew Murray wrote: >> On Thu, Nov 07, 2019 at 12:37:44AM +0100, Marek Vasut wrote: >>> On 10/26/19 10:36 PM, Andrew Murray wrote: >>> [...]>> 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.). >>> >>> I understand that Linux programs the endpoints correctly. However this >>> still doesn't prevent the endpoint from being broken and from sending a >>> transaction to that non-existent memory. >> >> Correct. >> >>> The PCI controller can prevent >>> that and in an automotive SoC, I would very much like the PCI controller >>> to do just that, rather than hope that the endpoint would always work. >> >> OK I understand - At least when working on the assumption that your RC will >> block RC->CPU transactions that are not described in any of it's windows. >> Thus you want to use the dma-ranges as a means to configure your controller >> to do this. > > Yes > >> What actually happens if you have a broken endpoint that reads/writes to >> non-existent memory on this hardware? Ideally the RC would generate a >> CA or UR back to the endpoint - does something else happen? Lockup, dead RC, >> performance issues? > > The behavior is undefined. > >> Using built-in features of the RC to prevent it from sending transactions >> to non-existent addresses is clearly helpful. But of course it doesn't stop >> a broken EP from writing to existent addresses, so only provides limited >> protection. > > Correct. > >> Despite the good intentions here, it doesn't seem like dma-ranges is >> designed for this purpose and as the hardware has limited ranges it will >> only be best-effort. > So what other options do we have ? If you really want to sacrifice DMA efficiency for a perceived increase in theoretical robustness by setting very conservative windows, then ultimately it's your choice, go ahead. It's just that you *need* to make that choice in the bootloader, not in Linux. If Linux gets passed dma-ranges that aren't actually reflected by the hardware, such that this patch is needed, then it *will* go wrong eventually, and you'll only get an "I told you so" from me. The bootloader knows what platform it's running on, so it has no excuse for emitting more ranges than there are available windows on that platform. Robin.
next prev parent reply index Thread overview: 52+ 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 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 [this message] 2019-12-22 7:46 ` Marek Vasut 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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.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
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 \ email@example.com 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