On Thu 27 Feb 18:57 PST 2020, Bjorn Andersson wrote: Rob, Will, we're reaching the point where upstream has enough functionality that this is becoming a critical issue for us. E.g. Lenovo Yoga C630 is lacking this and a single dts patch to boot mainline with display, GPU, WiFi and audio working and the story is similar on several devboards. As previously described, the only thing I want is the stream mapping related to the display controller in place, either with the CB with translation disabled or possibly with a way to specify the framebuffer region (although this turns out to mess things up in the display driver...) I did pick this up again recently and concluded that by omitting the streams for the USB controllers causes an instability issue seen on one of the controller to disappear. So I would prefer if we somehow could have a mechanism to only pick the display streams and the context allocation for this. Can you please share some pointers/insights/wishes for how we can conclude on this subject? PS. The list of devices specified https://lore.kernel.org/linux-arm-msm/cover.1587407458.git.saiprakash.ranjan@xxxxxxxxxxxxxx/ covers this need as well, if that matters... Thanks, Bjorn > On Mon 09 Dec 07:07 PST 2019, Thierry Reding wrote: > > > From: Thierry Reding <treding@xxxxxxxxxx> > > > > Sorry for the slow response on this, finally got the time to go through > this in detail and try it out on some Qualcomm boards. > > > On some platforms, the firmware will setup hardware to read from a given > > region of memory. One such example is a display controller that is > > scanning out a splash screen from physical memory. > > > > This particular use case is the one that we need to figure out for > Qualcomm devices as well; on some devices it's a simple splash screen > (that on many devices can be disabled), but for others we have EFIFB > on the display and no (sane) means to disable this. > > > During Linux' boot process, the ARM SMMU will configure all contexts to > > fault by default. This means that memory accesses that happen by an SMMU > > master before its driver has had a chance to properly set up the IOMMU > > will cause a fault. This is especially annoying for something like the > > display controller scanning out a splash screen because the faults will > > result in the display controller getting bogus data (all-ones on Tegra) > > and since it repeatedly scans that framebuffer, it will keep triggering > > such faults and spam the boot log with them. > > > > As my proposed patches indicated, the Qualcomm platform boots with > stream mapping setup for the hardware used by the bootloader, but > relying on the associated context banks not being enabled. > > USFCFG in SCR0 is set and any faults resulting of this will trap into > secure world and the device will be reset. > > > In order to work around such problems, scan the device tree for IOMMU > > masters and set up a special identity domain that will map 1:1 all of > > the reserved regions associated with them. This happens before the SMMU > > is enabled, so that the mappings are already set up before translations > > begin. > > > > One thing that was pointed out earlier, and which I don't have a good > > idea on how to solve it, is that the early identity domain is not > > discarded. The assumption is that the standard direct mappings code of > > the IOMMU framework will replace the early identity domain once devices > > are properly attached to domains, but we don't have a good point in time > > when it would be safe to remove the early identity domain. > > > > One option that I can think of would be to create an early identity > > domain for each master and inherit it when that master is attached to > > the domain later on, but that seems rather complicated from an book- > > keeping point of view and tricky because we need to be careful not to > > map regions twice, etc. > > > > The one concern I ran into with this approach (after resolving below > issues) is that when the display driver probes a new domain will be > created automatically and I get a stream of "Unhandled context fault" in > the log until the driver has mapped the framebuffer in the newly > allocated context. > > This is normally not a problem, as we seem to be able to do this > initialization in a few frames, but for the cases where the display > driver probe defer this is a problem. > > But at least these devices doesn't reboot, so this is way better than the > current state. > > > Any good ideas on how to solve this? It'd also be interesting to see if > > there's a more generic way of doing this. I know that something like > > this isn't necessary on earlier Tegra SoCs with the custom Tegra SMMU > > because translations are only enabled when the devices are attached to a > > domain. I'm not sure about other IOMMUs, but in the absence of a struct > > device, I suspect that we can't really do anything really generic that > > would work across drivers. > > > > As I indicated above I managed to get this working on the boards we have > that uses the arm-smmu driver. > > ## SDM845 > Booting the SDM845 shows the following register stream mapping register > content: > SMR(0): 0x80080880 S2CR(0): 0x0 > SMR(1): 0x80080c80 S2CR(1): 0x0 > SMR(2): 0x800f00a0 S2CR(2): 0x1 > SMR(3): 0x800f00c0 S2CR(3): 0x1 > SMR(4): 0x800f00e0 S2CR(4): 0x2 > SMR(5): 0x800f0100 S2CR(5): 0x2 > SMR(6): 0x0 S2CR(6): 0x0 > SMR(7): 0x0 S2CR(7): 0x0 > SMR(8): 0x0 S2CR(8): 0x200ff > SMR(9): 0x0 S2CR(9): 0x200ff > ... > > Here stream 0 and 1 (SID 0x880 and 0xc80) are the display streams, the > remainder are related to storage and USB - which afaict doesn't need to be > maintained. > > As the display uses context bank 0, using this as the identity bank results in > a couple of occurrences of: > Unhandled context fault: fsr=0x402, iova=0x9da00000, fsynr=0x370020, cbfrsynra=0x880, cb=0 > > Which we survive, but as we reach arm_smmu_device_reset() to flush out the new > stream mapping we start by writing S2CR(0) = 0, then SMR(0) = 0x800810a0. So > until SMR(4) is written we're lacking a valid stream mapping for the display, > and hence if the screen does refresh in during time period the device reboots. > > > In addition to this, the iommu_iova_to_phys() you perform in the mapping loop > results in a large number of "translation fault!" printouts from > arm_smmu_iova_to_phys_hard(). > > ## SM8150 > Boots with the following stream mapping: > SMR(0): 0x800006a0 S2CR(0): 0x0 > SMR(1): 0x800006c0 S2CR(1): 0x0 > SMR(2): 0x80000300 S2CR(2): 0x1 > SMR(3): 0x84200800 S2CR(3): 0x2 > SMR(4): 0x0 S2CR(4): 0x0 > SMR(5): 0x0 S2CR(5): 0x0 > SMR(6): 0x0 S2CR(6): 0x200ff > SMR(7): 0x0 S2CR(7): 0x200ff > ... > > Here stream 3 (sid 0x800) is the display stream. > > Mapping the various memory regions into the first context works fine, but > unless the display stream happens to be allocated to stream 3 (e.g. it always > ends up in slot 1 with my current DT) the board reboots shortly after we start > writing out the SMRs. I've not yet figured out why the board faults because of > the move to an earlier SMR index. (Perhaps because we clear the previously used > display SMR valid bit?) > > > ## Conclusions > Both of these platforms indicates that moving the stream mapping around is > going to cause issues, so inspired by my proposal I added below snippet right > before the call to arm_smmu_setup_identity(), in order to populate the stream > mapping selection. > > for (i = 0; i < smmu->num_mapping_groups; i++) { > smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(i)); > smmu->smrs[i].mask = FIELD_GET(ARM_SMMU_SMR_MASK, smr); > smmu->smrs[i].id = FIELD_GET(ARM_SMMU_SMR_ID, smr); > smmu->smrs[i].valid = !!(smr & ARM_SMMU_SMR_VALID); > } > > With this both boards boots fine, but I know Will had reservations wrt trusting > these values. Perhaps we could use the read back values (with some sanity > checking) only for setting up identity mapping? > > > With this I also tested booting MSM8996 (the db820c board) and except for > spending about 75 seconds printing below error in the kernel log during boot > things seems to be functional. > > [ 96.670723] arm-smmu b40000.iommu: translation fault! > [ 96.675038] arm-smmu b40000.iommu: PAR = 0x300000203 > > > Removing the call to iommu_iova_to_phys() in the mapping loop (as I know > that I don't have any memory regions with multiple clients) solves the > log spamming and all three boards seems to be functional. > > Regards, > Bjorn > > > Thierry > > > > Thierry Reding (2): > > iommu: arm-smmu: Extract arm_smmu_of_parse() > > iommu: arm-smmu: Add support for early direct mappings > > > > drivers/iommu/arm-smmu.c | 195 +++++++++++++++++++++++++++++++++++++-- > > drivers/iommu/arm-smmu.h | 2 + > > 2 files changed, 189 insertions(+), 8 deletions(-) > > > > -- > > 2.23.0 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
