On Tue, Aug 25, 2020 at 3:23 PM Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > > Hi, > > On Tue, Aug 25, 2020 at 12:01 PM Sai Prakash Ranjan > <saiprakash.ranjan@xxxxxxxxxxxxxx> wrote: > > > > Hi, > > > > On 2020-08-25 21:40, Doug Anderson wrote: > > > Hi, > > > > > > On Tue, Aug 25, 2020 at 8:43 AM Sai Prakash Ranjan > > > <saiprakash.ranjan@xxxxxxxxxxxxxx> wrote: > > >> > > >> Currently the non-strict or lazy mode of TLB invalidation can only be > > >> set > > >> for all or no domains. This works well for development platforms where > > >> setting to non-strict/lazy mode is fine for performance reasons but on > > >> production devices, we need a more fine grained control to allow only > > >> certain peripherals to support this mode where we can be sure that it > > >> is > > >> safe. So add support to filter non-strict/lazy mode based on the > > >> device > > >> names that are passed via cmdline parameter "iommu.nonstrict_device". > > >> > > >> Example: > > >> iommu.nonstrict_device="7c4000.sdhci,a600000.dwc3,6048000.etr" > > Just curious: are device names like this really guaranteed to be > stable across versions? > > > > > I have an inherent dislike of jamming things like this onto the > > > command line. IMHO the command line is the last resort for specifying > > > configuration and generally should be limited to some specialized > > > debug options and cases where the person running the kernel needs to > > > override a config that was set by the person (or company) compiling > > > the kernel. Specifically, having a long/unwieldy command line makes > > > it harder to use for the case when an end user actually wants to use > > > it to override something. It's also just another place to look for > > > config. > > > > > > > Good thing about command line parameters are that they are optional, > > they do > > not specify any default behaviour (I mean they are not mandatory to be > > set > > for the system to be functional), so I would like to view it as an > > optional > > config. And this command line parameter (nonstrict_device) is strictly > > optional > > with default being strict already set in the driver. > > > > They can be passed from the bootloader via chosen node for DT platforms > > or choose > > a new *bootconfig* as a way to pass the cmdline but finally it does boil > > down to > > just another config. > > Never looked at bootconfig. Unfortunately it seems to require > initramfs so that pretty much means it's out for my usage. :( > > > > I agree with general boolean or single value command line parameters > > being just > > more messy which could just be Kconfigs instead but for multiple value > > parameters > > like these do not fit in Kconfig. > > > > As you might already know, command line also gives an advantage to the > > end user > > to configure system without building kernel, for this specific command > > line its > > very useful because the performance bump is quite noticeable when the > > iommu.strict > > is off. Now for end user who would not be interested in building entire > > kernel(majority) > > and just cares about good speeds or throughput can find this very > > beneficial. > > I am not talking about one specific OS usecase here but more in general > > term. > > > > > The other problem is that this doesn't necessarily scale very well. > > > While it works OK for embedded cases it doesn't work terribly well for > > > distributions. I know that in an out-of-band thread you indicated > > > that it doesn't break anything that's not already broken (AKA this > > > doesn't fix the distro case but it doesn't make it worse), it would be > > > better to come up with a more universal solution. > > > > > > > Is the universal solution here referring to fix all the command line > > parameters > > in the kernel or this specific command line? Are we going to remove any > > more > > addition to the cmdline ;) > > There are very few cases where a kernel command line parameter is the > only way to configure something. Most of the time it's just there to > override a config. I wouldn't suggest removing those. I just don't > want a kernel command line parameter to be the primary way to enable > something. > > > > So possible other solution is the *bootconfig* which is again just > > another place > > to look for a config. So thing is that this universal solution would > > result in > > just more new fancy ways of passing configs or adding such configs to > > the drivers > > or subsystems in kernel which is pretty much similar to implementing > > policy in > > kernel which I think is frowned upon and mentioned in the other thread. > > > > > Ideally it feels like we should figure out how to tag devices in a > > > generic manner automatically (hardcode at the driver or in the device > > > tree). I think the out-of-band discussions talked about "external > > > facing" and the like. We could also, perhaps, tag devices that have > > > "binary blob" firmware if we wanted. Then we'd have a policy (set by > > > Kconfig, perhaps overridable via commandline) that indicated the > > > strictness level for the various classes of devices. So policy would > > > be decided by KConfig and/or command line. > > > > > > > How is tagging in driver or device tree better than the simple command > > line > > approach to pass the same list of devices which otherwise you would > > hardcode > > in the corresponding drivers and device tree all over the kernel other > > than > > the scalability part for command line? IMHO it is too much churn. > > It's better because it doesn't require keeping track and updating > these per-board (or per machine) arguments for each and every > board/machine you maintain. If, for instance, we start out by > allowing HW video decoder to use non-strict. So: > > On one board, we add in "aa00000.video-codec" to the command line. > On some other board, maybe we add in "1d00000.video-codec" to the command line. > On some other board, maybe we add in "90400000.video-codec" to the command line. > > Now we realize that there's some problem and we have to remove it, so > we need to go through and remove this from our command line > everywhere. Worse is that we have to proactively notice it and remove > it. > > Instead, let's imagine that we set a policy at a bit of a higher > level. Different ideas: > > a) We could have a CONFIG_ option for the video codec that's something > like "CONFIG_VIDEOCODEC_DEFAULT_NONSTRICT". If this was set then if > there is no "iommu.strict" command line argument then this device > would be mapped as non-strict. If "iommu.strict=0" or > "iommu.strict=1" is on the command line then it would override all of > these defaults. Probably the existence (or maybe the default value) > of this CONFIG option implies that there are no known/expected > exploits related to it. jumping in a bit late so might have missed some of the background, but shouldn't the driver decide strict vs non-strict? It should be the one that knows if non-strict is safe or not? BR, -R > > b) We could find some way to tag the video codec and then set > non-strictness on certain classes of devices, then we could have a > policy to disable strictness on certain classes of devices. > > The nice thing about the above is that you could imagine someone > pushing a change to the stable trees that would fix everyone affected. > Nobody would need to go around and adjust command line options, they'd > just get the newest stable and it could cause devices to move into > strict mode if there was a known exploit. I suppose with your > proposal stable trees could have a "blacklist" where the commandline > is ignored for exploited devices, but that seems ugly. > > > > Device tree could be used but then we have a problem with it being for > > only > > describing hardware and it doesn't work for ACPI based systems. > > > > Command line approach works for all systems (both DT and ACPI) without > > having > > to add too much churn to drivers. Lastly, I think we can have both > > options, it > > doesn't hurt to add command line parameter since it is optional. > > I'm not opposed to something existing that lets you override this on > the command line, but I'm just not a fan of it being the primary way. > > -Doug > _______________________________________________ > iommu mailing list > iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx > https://lists.linuxfoundation.org/mailman/listinfo/iommu
