From: Thierry Reding <thierry.reding@gmail.com>
To: Will Deacon <will@kernel.org>
Cc: devicetree@vger.kernel.org, Robin Murphy <robin.murphy@arm.com>,
linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
Rob Herring <robh+dt@kernel.org>,
Frank Rowand <frowand.list@gmail.com>
Subject: Re: [PATCH 2/2] iommu: dma: Use of_iommu_get_resv_regions()
Date: Wed, 27 Nov 2019 15:16:31 +0100 [thread overview]
Message-ID: <20191127141631.GA280099@ulmo> (raw)
In-Reply-To: <20191126172910.GA2669319@ulmo>
[-- Attachment #1.1: Type: text/plain, Size: 10764 bytes --]
On Wed, Nov 27, 2019 at 01:16:54PM +0100, Thierry Reding wrote:
> On Tue, Sep 17, 2019 at 06:59:50PM +0100, Will Deacon wrote:
> > On Mon, Sep 02, 2019 at 04:52:45PM +0200, Thierry Reding wrote:
> > > On Mon, Sep 02, 2019 at 03:22:35PM +0100, Robin Murphy wrote:
> > > > On 29/08/2019 12:14, Thierry Reding wrote:
> > > > > From: Thierry Reding <treding@nvidia.com>
> > > > >
> > > > > For device tree nodes, use the standard of_iommu_get_resv_regions()
> > > > > implementation to obtain the reserved memory regions associated with a
> > > > > device.
> > > >
> > > > This covers the window between iommu_probe_device() setting up a default
> > > > domain and the device's driver finally probing and taking control, but
> > > > iommu_probe_device() represents the point that the IOMMU driver first knows
> > > > about this device - there's still a window from whenever the IOMMU driver
> > > > itself probed up to here where the "unidentified" traffic may have already
> > > > been disrupted. Some IOMMU drivers have no option but to make the necessary
> > > > configuration during their own probe routine, at which point a struct device
> > > > for the display/etc. endpoint may not even exist yet.
> > >
> > > Yeah, I think I'm actually running into this issue with the ARM SMMU
> > > driver. The above works fine with the Tegra SMMU driver, though, because
> > > it doesn't touch the SMMU configuration until a device is attached to a
> > > domain.
> > >
> > > For anything earlier than iommu_probe_device(), I don't see a way of
> > > doing this generically. I've been working on a prototype to make these
> > > reserved memory regions early on for ARM SMMU but I've been failing so
> > > far. I think it would possibly work if we just switched the default for
> > > stream IDs to be "bypass" if they have any devices that have reserved
> > > memory regions, but again, this isn't quite working (yet).
> >
> > I think we should avoid the use of "bypass" outside of the IOMMU probe()
> > routine if at all possible, since it leaves the thing wide open if we don't
> > subsequently probe the master.
> >
> > Why can't we initialise a page-table early for StreamIDs with these
> > reserved regions, and then join that up later on if we see a device with
> > one of those StreamIDs attaching to a DMA domain? I suppose the nasty
> > case would be attaching to a domain that already has other masters
> > attached to it. Can we forbid that somehow for these devices? Otherwise,
> > I think we'd have to transiently switch to bypass whilst switching page
> > table.
> >
> > What problems did you run into trying to implement this?
>
> I picked this up again and was trying to make this work with your
> suggestion. Below is a rough draft and it seems to be working to some
> degree. Here's an extract of the log when I run this on Jetson TX2:
>
> [ 3.755328] arm-smmu 12000000.iommu: probing hardware configuration...
> [ 3.762187] arm-smmu 12000000.iommu: SMMUv2 with:
> [ 3.767137] arm-smmu 12000000.iommu: stage 1 translation
> [ 3.772806] arm-smmu 12000000.iommu: stage 2 translation
> [ 3.778471] arm-smmu 12000000.iommu: nested translation
> [ 3.784050] arm-smmu 12000000.iommu: stream matching with 128 register groups
> [ 3.791651] arm-smmu 12000000.iommu: 64 context banks (0 stage-2 only)
> [ 3.798603] arm-smmu 12000000.iommu: Supported page sizes: 0x61311000
> [ 3.805460] arm-smmu 12000000.iommu: Stage-1: 48-bit VA -> 48-bit IPA
> [ 3.812310] arm-smmu 12000000.iommu: Stage-2: 48-bit IPA -> 48-bit PA
> [ 3.819159] arm-smmu 12000000.iommu: > arm_smmu_setup_identity(smmu=ffff0001eabcec80)
> [ 3.827373] arm-smmu 12000000.iommu: identity domain: ffff0001eaf8cae8 (ops: 0x0)
> [ 3.835614] arm-smmu 12000000.iommu: np: /ethernet@2490000
> [ 3.841635] arm-smmu 12000000.iommu: np: /sdhci@3400000
> [ 3.847315] arm-smmu 12000000.iommu: np: /sdhci@3420000
> [ 3.852990] arm-smmu 12000000.iommu: np: /sdhci@3440000
> [ 3.858683] arm-smmu 12000000.iommu: np: /sdhci@3460000
> [ 3.864370] arm-smmu 12000000.iommu: np: /hda@3510000
> [ 3.869897] arm-smmu 12000000.iommu: np: /usb@3530000
> [ 3.875421] arm-smmu 12000000.iommu: np: /pcie@10003000
> [ 3.881109] arm-smmu 12000000.iommu: np: /host1x@13e00000
> [ 3.887012] arm-smmu 12000000.iommu: np: /host1x@13e00000/display-hub@15200000/display@15200000
> [ 3.896344] arm-smmu 12000000.iommu: region: /reserved-memory/framebuffer@9607c000
> [ 3.904707] arm-smmu 12000000.iommu: [mem 0x9607c000-0x9687bfff]
> [ 3.915719] arm-smmu 12000000.iommu: /iommu@12000000: 1 arguments
> [ 3.922487] arm-smmu 12000000.iommu: 0: 00000009
> [ 3.927888] arm-smmu 12000000.iommu: SID: 0009 MASK: 7f80
> [ 3.934132] arm-smmu 12000000.iommu: found index: 0
> [ 3.939840] arm-smmu 12000000.iommu: np: /host1x@13e00000/display-hub@15200000/display@15210000
> [ 3.949183] arm-smmu 12000000.iommu: np: /host1x@13e00000/display-hub@15200000/display@15220000
> [ 3.958499] arm-smmu 12000000.iommu: np: /host1x@13e00000/vic@15340000
> [ 3.965557] arm-smmu 12000000.iommu: np: /gpu@17000000
> [ 3.971145] arm-smmu 12000000.iommu: np: /bpmp
> [ 3.976084] arm-smmu 12000000.iommu: < arm_smmu_setup_identity()
> [ 3.982613] arm-smmu 12000000.iommu: > arm_smmu_write_sme(smmu=ffff0001eabcec80, idx=0)
> [ 3.991072] arm-smmu 12000000.iommu: ARM_SMMU_GR0_S2CR(0) < 00020000
> [ 3.997922] arm-smmu 12000000.iommu: ARM_SMMU_GR0_SMR(0) < ff800009
> [ 4.004677] arm-smmu 12000000.iommu: < arm_smmu_write_sme()
> [ 4.010528] arm-smmu 12000000.iommu: > arm_smmu_write_sme(smmu=ffff0001eabcec80, idx=1)
> [ 4.018919] arm-smmu 12000000.iommu: ARM_SMMU_GR0_S2CR(1) < 00020000
> [ 4.025773] arm-smmu 12000000.iommu: ARM_SMMU_GR0_SMR(1) < 00000000
> [ 4.032543] arm-smmu 12000000.iommu: < arm_smmu_write_sme()
> ...
>
> There's a bunch of these, but idx=0 is the only one that's actually
> populated because it corresponds to the display controller. However,
> shortly after this I see a bunch of these:
>
> ...
> [ 7.588908] arm-smmu 12000000.iommu: Blocked unknown Stream ID 0x809; boot with "arm-smmu.disable_bypass=0" to allow, but this may have security implications
> [ 7.589907] arm-smmu: > arm_smmu_of_xlate(dev=ffff0001eaecf010, args=ffff80001024bae8)
> [ 7.603599] arm-smmu 12000000.iommu: GFSR 0x80000002, GFSYNR0 0x00000000, GFSYNR1 0x00000809, GFSYNR2 0x00000000
> [ 7.604218] arm-smmu 12000000.iommu: Blocked unknown Stream ID 0x1409; boot with "arm-smmu.disable_bypass=0" to allow, but this may have security implications
> [ 7.611956] arm-smmu: < arm_smmu_of_xlate() = 0
> [ 7.622636] arm-smmu 12000000.iommu: GFSR 0x80000002, GFSYNR0 0x00000000, GFSYNR1 0x00001409, GFSYNR2 0x00000000
> [ 7.622658] arm-smmu 12000000.iommu: Blocked unknown Stream ID 0x1809; boot with "arm-smmu.disable_bypass=0" to allow, but this may have security implications
> [ 7.622662] arm-smmu 12000000.iommu: GFSR 0x80000002, GFSYNR0 0x00000000, GFSYNR1 0x00001809, GFSYNR2 0x00000000
> [ 7.622676] arm-smmu 12000000.iommu: Blocked unknown Stream ID 0x409; boot with "arm-smmu.disable_bypass=0" to allow, but this may have security implications
> [ 7.637739] arm-smmu 12000000.iommu: ARM_SMMU_GR0_S2CR(1) < 00000001
> [ 7.642199] arm-smmu 12000000.iommu: GFSR 0x80000002, GFSYNR0 0x00000000, GFSYNR1 0x00000409, GFSYNR2 0x00000000
> [ 7.642216] arm-smmu 12000000.iommu: Blocked unknown Stream ID 0x9; boot with "arm-smmu.disable_bypass=0" to allow, but this may have security implications
> [ 7.642221] arm-smmu 12000000.iommu: GFSR 0x80000002, GFSYNR0 0x00000000, GFSYNR1 0x00000009, GFSYNR2 0x00000000
> [ 7.642237] arm-smmu 12000000.iommu: Blocked unknown Stream ID 0x1c09; boot with "arm-smmu.disable_bypass=0" to allow, but this may have security implications
> [ 7.652992] tegra-host1x 13e00000.host1x: Adding to iommu group 0
> [ 7.667720] arm-smmu 12000000.iommu: GFSR 0x80000002, GFSYNR0 0x00000000, GFSYNR1 0x00001c09, GFSYNR2 0x00000000
> [ 7.667732] arm-smmu 12000000.iommu: Blocked unknown Stream ID 0x9; boot with "arm-smmu.disable_bypass=0" to allow, but this may have security implications
> [ 7.667736] arm-smmu 12000000.iommu: GFSR 0x80000002, GFSYNR0 0x00000000, GFSYNR1 0x00000009, GFSYNR2 0x00000000
> [ 7.667748] arm-smmu 12000000.iommu: Blocked unknown Stream ID 0x1809; boot with "arm-smmu.disable_bypass=0" to allow, but this may have security implications
> [ 7.667752] arm-smmu 12000000.iommu: GFSR 0x80000002, GFSYNR0 0x00000000, GFSYNR1 0x00001809, GFSYNR2 0x00000000
> [ 7.667765] arm-smmu 12000000.iommu: Blocked unknown Stream ID 0x9; boot with "arm-smmu.disable_bypass=0" to allow, but this may have security implications
> [ 7.678511] arm-smmu 12000000.iommu: > arm_smmu_write_sme(smmu=ffff0001eabcec80, idx=1)
> [ 7.693158] arm-smmu 12000000.iommu: GFSR 0x80000002, GFSYNR0 0x00000000, GFSYNR1 0x00000009, GFSYNR2 0x00000000
> [ 7.693170] arm-smmu 12000000.iommu: Blocked unknown Stream ID 0x1009; boot with "arm-smmu.disable_bypass=0" to allow, but this may have security implications
> [ 7.693174] arm-smmu 12000000.iommu: GFSR 0x80000002, GFSYNR0 0x00000000, GFSYNR1 0x00001009, GFSYNR2 0x00000000
>
> Note that stream ID 0x9 is TEGRA186_SID_NVDISPLAY, which is associated
> with the display controllers. One of these display controllers is live
> because it was turned on by the bootloader to show a splash screen.
>
> What I don't really understand is why it thinks that that stream ID is
> unknown. One possibility I see is that perhaps the S2CR(0) and/or SMR(0)
> registers might have gotten overwritten, but I don't see where that may
> happen.
>
> The errors stop eventually when the display controller is hooked up
> properly via the DMA API, but the whole purpose here is obviously to get
> to that point much earlier.
>
> Any ideas what I might be doing wrong? Any comments on the general
> approach?
Nevermind that, I figured out that I was missingthe initialization of
some of the S2CR variables. I've got something that I think is working
now, though I don't know yet how to go about cleaning up the initial
mapping and "recycling" it.
I'll clean things up a bit, run some more tests and post a new patch
that can serve as a basis for discussion.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 156 bytes --]
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2019-11-27 14:16 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-29 11:14 [PATCH 0/2] iommu: Support reserved-memory regions Thierry Reding
2019-08-29 11:14 ` [PATCH 1/2] iommu: Implement of_iommu_get_resv_regions() Thierry Reding
2019-09-02 13:38 ` Rob Herring
2019-09-02 13:54 ` Robin Murphy
2019-09-02 15:00 ` Thierry Reding
2019-09-09 14:04 ` Will Deacon
2019-08-29 11:14 ` [PATCH 2/2] iommu: dma: Use of_iommu_get_resv_regions() Thierry Reding
2019-09-02 14:22 ` Robin Murphy
2019-09-02 14:52 ` Thierry Reding
2019-09-17 17:59 ` Will Deacon
2019-11-27 12:16 ` Thierry Reding
2019-11-27 14:16 ` Thierry Reding [this message]
2019-11-27 17:09 ` Robin Murphy
2019-11-28 11:43 ` Thierry Reding
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=20191127141631.GA280099@ulmo \
--to=thierry.reding@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=frowand.list@gmail.com \
--cc=iommu@lists.linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=robin.murphy@arm.com \
--cc=will@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).