| Message ID | 20210624171759.4125094-1-dianders@chromium.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | iommu: Enable non-strict DMA on QCom SD/MMC | expand |
Hi Douglas, On Thu, Jun 24, 2021 at 10:17:56AM -0700, Douglas Anderson wrote: > The goal of this patch series is to get better SD/MMC performance on > Qualcomm eMMC controllers and in generally nudge us forward on the > path of allowing some devices to be in strict mode and others to be in > non-strict mode. So if I understand it right, this patch-set wants a per-device decision about setting dma-mode to strict vs. non-strict. I think we should get to the reason why strict mode is used by default first. Is the default on ARM platforms to use iommu-strict mode by default and if so, why? The x86 IOMMUs use non-strict mode by default (yes, it is a security trade-off). Regards, Joerg
Hi, On Fri, Jun 25, 2021 at 6:19 AM Joerg Roedel <joro@8bytes.org> wrote: > > Hi Douglas, > > On Thu, Jun 24, 2021 at 10:17:56AM -0700, Douglas Anderson wrote: > > The goal of this patch series is to get better SD/MMC performance on > > Qualcomm eMMC controllers and in generally nudge us forward on the > > path of allowing some devices to be in strict mode and others to be in > > non-strict mode. > > So if I understand it right, this patch-set wants a per-device decision > about setting dma-mode to strict vs. non-strict. > > I think we should get to the reason why strict mode is used by default > first. Is the default on ARM platforms to use iommu-strict mode by > default and if so, why? > > The x86 IOMMUs use non-strict mode by default (yes, it is a security > trade-off). It is certainly a good question. I will say that, as per usual, I'm fumbling around trying to solve problems in subsystems I'm not an expert at, so if something I'm saying sounds like nonsense it probably is. Please correct me. I guess I'd start out by thinking about what devices I think need to be in "strict" mode. Most of my thoughts on this are in the 3rd patch in the series. I think devices where it's important to be in strict mode fall into "Case 1" from that patch description, copied here: Case 1: IOMMUs prevent malicious code running on the peripheral (maybe a malicious peripheral or maybe someone exploited a benign peripheral) from turning into an exploit of the Linux kernel. This is particularly important if the peripheral has loadable / updatable firmware or if the peripheral has some type of general purpose processor and is processing untrusted inputs. It's also important if the device is something that can be easily plugged into the host and the device has direct DMA access itself, like a PCIe device. Using sc7180 as an example (searching for iommus in sc7180.dtsi), I'd expect these peripherals to be in strict mode: * WiFi / LTE - I'm almost certain we want this in "strict" mode. Both have loadable / updatable firmware and both do complex processing on untrusted inputs. Both have a history of being compromised over the air just by being near an attacker. Note that on sc7180 these are _not_ connected over PCI so we can't leverage any PCI mechanism for deciding strict / non-strict. * Video decode / encode - pretty sure we want this in strict. It's got loadable / updatable firmware and processing complex / untrusted inputs. * LPASS (low power audio subsystem) - I don't know a ton and I think we don't use this much on our designs, but I believe it meets the definitions for needing "strict". * The QUPs (handles UART, SPI, and i2c) - I'm not as sure here. These are much "smarter" than you'd expect. They have loadable / updatable firmware and certainly have a sort of general purpose processor in them. They also might be processing untrusted inputs, but presumably in a pretty simple way. At the moment we don't use a ton of DMA here anyway and these are pretty low speed, so I would tend to leave them as strict just to be on the safe side. I'd expect these to be non-strict: * SD/MMC - as described in this patch series. * USB - As far as I know firmware isn't updatable and has no history of being compromised. Special: * GPU - This already has a bunch of special cases, so we probably don't need to discuss here. As far as I can tell everything in sc7180.dtsi that has an "iommus" property is classified above. So, unless I'm wrong and it's totally fine to run LTE / WiFi / Video / LPASS in non-strict mode then: * We still need some way to pick strict vs. non-strict. * Since I've only identified two peripherals that I think should be non-strict, having "strict" the default seems like fewer special cases. It's also safer. In terms of thinking about x86 / AMD where the default is non-strict, I don't have any historical knowledge there. I guess the use of PCI for connecting WiFi is more common (so you can use the PCI special cases) and I'd sorta hope that WiFi is running in strict mode. For video encode / decode, perhaps x86 / AMD are just accepting the risk here because there was no kernel infrastructure for doing better? I'd also expect that x86/AMD don't have something quite as crazy as the QUPs for UART/I2C/SPI, but even if they do I wouldn't be terribly upset if they were in non-strict mode. ...so I guess maybe the super short answer to everything above is that I believe that at least WiFi ought to be in "strict" mode and it's not on PCI so we need to come up with some type of per-device solution. -Doug
Hi, On Fri, Jun 25, 2021 at 7:42 AM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Fri, Jun 25, 2021 at 6:19 AM Joerg Roedel <joro@8bytes.org> wrote: > > > > Hi Douglas, > > > > On Thu, Jun 24, 2021 at 10:17:56AM -0700, Douglas Anderson wrote: > > > The goal of this patch series is to get better SD/MMC performance on > > > Qualcomm eMMC controllers and in generally nudge us forward on the > > > path of allowing some devices to be in strict mode and others to be in > > > non-strict mode. > > > > So if I understand it right, this patch-set wants a per-device decision > > about setting dma-mode to strict vs. non-strict. > > > > I think we should get to the reason why strict mode is used by default > > first. Is the default on ARM platforms to use iommu-strict mode by > > default and if so, why? > > > > The x86 IOMMUs use non-strict mode by default (yes, it is a security > > trade-off). > > It is certainly a good question. I will say that, as per usual, I'm > fumbling around trying to solve problems in subsystems I'm not an > expert at, so if something I'm saying sounds like nonsense it probably > is. Please correct me. > > I guess I'd start out by thinking about what devices I think need to > be in "strict" mode. Most of my thoughts on this are in the 3rd patch > in the series. I think devices where it's important to be in strict > mode fall into "Case 1" from that patch description, copied here: > > Case 1: IOMMUs prevent malicious code running on the peripheral (maybe > a malicious peripheral or maybe someone exploited a benign peripheral) > from turning into an exploit of the Linux kernel. This is particularly > important if the peripheral has loadable / updatable firmware or if > the peripheral has some type of general purpose processor and is > processing untrusted inputs. It's also important if the device is > something that can be easily plugged into the host and the device has > direct DMA access itself, like a PCIe device. > > > Using sc7180 as an example (searching for iommus in sc7180.dtsi), I'd > expect these peripherals to be in strict mode: > > * WiFi / LTE - I'm almost certain we want this in "strict" mode. Both > have loadable / updatable firmware and both do complex processing on > untrusted inputs. Both have a history of being compromised over the > air just by being near an attacker. Note that on sc7180 these are > _not_ connected over PCI so we can't leverage any PCI mechanism for > deciding strict / non-strict. > > * Video decode / encode - pretty sure we want this in strict. It's got > loadable / updatable firmware and processing complex / untrusted > inputs. > > * LPASS (low power audio subsystem) - I don't know a ton and I think > we don't use this much on our designs, but I believe it meets the > definitions for needing "strict". > > * The QUPs (handles UART, SPI, and i2c) - I'm not as sure here. These > are much "smarter" than you'd expect. They have loadable / updatable > firmware and certainly have a sort of general purpose processor in > them. They also might be processing untrusted inputs, but presumably > in a pretty simple way. At the moment we don't use a ton of DMA here > anyway and these are pretty low speed, so I would tend to leave them > as strict just to be on the safe side. > > > I'd expect these to be non-strict: > > * SD/MMC - as described in this patch series. > > * USB - As far as I know firmware isn't updatable and has no history > of being compromised. > > > Special: > > * GPU - This already has a bunch of special cases, so we probably > don't need to discuss here. > > > As far as I can tell everything in sc7180.dtsi that has an "iommus" > property is classified above. So, unless I'm wrong and it's totally > fine to run LTE / WiFi / Video / LPASS in non-strict mode then: > > * We still need some way to pick strict vs. non-strict. > > * Since I've only identified two peripherals that I think should be > non-strict, having "strict" the default seems like fewer special > cases. It's also safer. > > > In terms of thinking about x86 / AMD where the default is non-strict, > I don't have any historical knowledge there. I guess the use of PCI > for connecting WiFi is more common (so you can use the PCI special > cases) and I'd sorta hope that WiFi is running in strict mode. For > video encode / decode, perhaps x86 / AMD are just accepting the risk > here because there was no kernel infrastructure for doing better? I'd > also expect that x86/AMD don't have something quite as crazy as the > QUPs for UART/I2C/SPI, but even if they do I wouldn't be terribly > upset if they were in non-strict mode. > > ...so I guess maybe the super short answer to everything above is that > I believe that at least WiFi ought to be in "strict" mode and it's not > on PCI so we need to come up with some type of per-device solution. I guess this thread has been silent for a bit of time now. Given that my previous version generated a whole bunch of traffic, I guess I'm assuming this: a) Nothing is inherently broken with my current approach. b) My current approach doesn't make anybody terribly upset even if nobody is totally in love with it. c) Nobody has any other bright ideas for ways to solve this that would make my patch series obsolete. I guess I'll take that as a good sign and hope that it means that this approach has a path forward. I suppose it could just be that everyone is busy and/or on vacation, but I've always been an optimist! My plan continues to be to send a v3 of my patch series atop Sai's patch [1] and John's series [2]. I'll plan to wait a bit longer before posting my v3 to allow for more feedback/thoughts and also to see if either Sai's patches or John's patches land and/or have newer versions posted. :-) -Doug [1] https://lore.kernel.org/r/20210623134201.16140-1-saiprakash.ranjan@codeaurora.org [2] https://lore.kernel.org/linux-doc/1624016058-189713-1-git-send-email-john.garry@huawei.com
On Wed, Jul 07, 2021 at 01:00:13PM -0700, Doug Anderson wrote: > a) Nothing is inherently broken with my current approach. > > b) My current approach doesn't make anybody terribly upset even if > nobody is totally in love with it. Well, no, sorry :) I don't think it is a good idea to allow drivers to opt-out of the strict-setting. This is a platform or user decision, and the driver should accept whatever it gets. So the real question is still why strict is the default setting and how to change that. Or document for the users that want performance how to change the setting, so that they can decide. Regards, Joerg
Hi, On Thu, Jul 8, 2021 at 1:09 AM Joerg Roedel <joro@8bytes.org> wrote: > > On Wed, Jul 07, 2021 at 01:00:13PM -0700, Doug Anderson wrote: > > a) Nothing is inherently broken with my current approach. > > > > b) My current approach doesn't make anybody terribly upset even if > > nobody is totally in love with it. > > Well, no, sorry :) > > I don't think it is a good idea to allow drivers to opt-out of the > strict-setting. This is a platform or user decision, and the driver > should accept whatever it gets. Sure, I agree with you there. The driver shouldn't ever be able to override and make things less strict than the user or platform wants. It feels like that can be accomplished. See below. > So the real question is still why strict is the default setting and how > to change that. I guess there are two strategies if we agree that there's a benefit to running some devices in strict and others in non-strict: * opt-in to strict: default is non-strict and we have to explicitly list what we want to be strict. * opt-out of strict: default is strict and we have to explicitly list what we want to be non-strict. I guess the question is: do we allow both strategies or only one of them? I think you are suggesting that the kernel should support "opt-in" to strict and that that matches the status quo with PCI on x86. I'm pushing for some type of "opt-out" of strict support. I have heard from security folks that they'd prefer "opt-out" of strict as well. If we're willing to accept more complex config options we could support both choosable by KConfig. How it'd all work in my mind: Command line: * iommu.strict=0 - suggest non-strict by default * iommu.strict=1 - force strict for all drivers * iommu.strict not specified - no opinion Kconfig: * IOMMU_DEFAULT_LAZY - suggest non-strict by default; drivers can opt-in to strict * IOMMU_DEFAULT_STRICT - force strict for all drivers * IOMMU_DEFAULT_LOOSE_STRICT - allow explicit suggestions for laziness but default to strict if no votes. Drivers: * suggest lazy - suggest non-strict * force strict - force strict * no vote How the above work together: * if _any_ of the three things wants strict then it's strict. * if _all_ of the three things want lazy then it's lazy. * If the KConfig is "loose strict" and the command line is set to "lazy" then it's equivalent to the KConfig saying "lazy". In other words drivers could still "opt-in" to strict but otherwise we'd be lazy. * The only way for a driver's "suggest lazy" vote to have any effect at all is if "iommu.strict" wasn't specified on the command line _and_ if the KConfig was "loose strict". This is effectively the "opt-out" of lazy. If you think the strategy I describe above is garbage then would you be OK if I re-worked my patchset to at least allow non-PCI drivers to "opt-in" to strict? Effectively I'd change patch #3 to list all of the peripherals on my SoC _except_ the USB and SD/MMC and request that they all be strict. If other people expressed their preference for the "opt-out" of strict strategy would that change your mind? > Or document for the users that want performance how to > change the setting, so that they can decide. Pushing this to the users can make sense for a Linux distribution but probably less sense for an embedded platform. So I'm happy to make some way for a user to override this (like via kernel command line), but I also strongly believe there should be a default that users don't have to futz with that we think is correct. -Doug
On 2021-07-08 09:08, Joerg Roedel wrote: > On Wed, Jul 07, 2021 at 01:00:13PM -0700, Doug Anderson wrote: >> a) Nothing is inherently broken with my current approach. >> >> b) My current approach doesn't make anybody terribly upset even if >> nobody is totally in love with it. > > Well, no, sorry :) > > I don't think it is a good idea to allow drivers to opt-out of the > strict-setting. This is a platform or user decision, and the driver > should accept whatever it gets. > > So the real question is still why strict is the default setting and how > to change that. Or document for the users that want performance how to > change the setting, so that they can decide. As I mentioned before, conceptually I think this very much belongs in sysfs as a user decision. We essentially have 4 levels of "strictness": 1: DMA domain with bounce pages 2: DMA domain 3: DMA domain with flush queue 4: Identity domain The "make this device go faster because I trust it" use-case is why we have the sysfs interface to switch between 2 and 4, so it's entirely logical to have the intermediate option as well for when 3 is "faster" enough while still giving a bit more peace of mind than full-on bypass. Making it a platform-specific decision that's hidden in a driver - arm-smmu-qcom can be considered a dumping ground of detailed platform knowledge ;) - happens to work as a reasonable compromise for this specific case, but I concur that it could be viewed as setting a precedent for other cases which we definitely aren't as reasonable. I've been thinking about the sysfs thing some more, and since it's Friday afternoon and I can't concentrate on what I'm supposed to be doing anyway, let's see how far I can get by Monday... Robin.
On 2021-07-08 09:08, Joerg Roedel wrote: > On Wed, Jul 07, 2021 at 01:00:13PM -0700, Doug Anderson wrote: >> a) Nothing is inherently broken with my current approach. >> >> b) My current approach doesn't make anybody terribly upset even if >> nobody is totally in love with it. > > Well, no, sorry :) > > I don't think it is a good idea to allow drivers to opt-out of the > strict-setting. This is a platform or user decision, and the driver > should accept whatever it gets. > > So the real question is still why strict is the default setting and how > to change that. It's occurred to me whilst hacking on the relevant area that there's an important point I may have somewhat glossed over there: most of the IOMMU drivers that are used for arm64 do not take advantage of non-strict mode anyway. If anything it would be detrimental, since iommu-dma would waste a bunch of time and memory managing flush queues and firing off the batch invalidations while internally the drivers are still invalidating each unmap synchronously. Those IOMMUs in mobile and embedded SoCs are also mostly used for media devices, where the buffers are relatively large and change relatively infrequently, so they are less likely to gain significantly from supporting non-strict mode. It's primarily the Arm SMMUs which get used in the more "x86-like" paradigm (especially in larger systems) of being stuck in front of everything including networking/storage/PCIe/etc. where the workloads are far more varied. Robin. > Or document for the users that want performance how to > change the setting, so that they can decide. > > Regards, > > Joerg > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu >
On 2021-07-08 15:36, Doug Anderson wrote: [...] >> Or document for the users that want performance how to >> change the setting, so that they can decide. > > Pushing this to the users can make sense for a Linux distribution but > probably less sense for an embedded platform. So I'm happy to make > some way for a user to override this (like via kernel command line), > but I also strongly believe there should be a default that users don't > have to futz with that we think is correct. FYI I did make progress on the "punt it to userspace" approach. I'm not posting it even as an RFC yet because I still need to set up a machine to try actually testing any of it (it's almost certainly broken somewhere), but in the end it comes out looking surprisingly not too bad overall. If you're curious to take a look in the meantime I put it here: https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/fq Cheers, Robin.
Hi Robin, On Fri, Jul 09, 2021 at 02:56:47PM +0100, Robin Murphy wrote: > As I mentioned before, conceptually I think this very much belongs in sysfs > as a user decision. We essentially have 4 levels of "strictness": > > 1: DMA domain with bounce pages > 2: DMA domain > 3: DMA domain with flush queue > 4: Identity domain Together with reasonable defaults (influenced by compile-time options) it seems to be a good thing to configure at runtime via sysfs. We already have CONFIG_IOMMU_DEFAULT_PASSTHROUGH, which can probably be extended to be an option list: - CONFIG_IOMMU_DEFAULT_PASSTHROUGH: Trusted devices are identity mapped - CONFIG_IOMMU_DEFAULT_DMA_STRICT: Trusted devices are DMA mapped with strict flush behavior on unmap - CONFIG_IOMMU_DEFAULT_DMA_LAZY: Trusted devices are DMA mapped with flush queues for performance Untrusted devices always get into the DMA domain with bounce pages by default. The defaults can be changed at runtime via sysfs. We already have basic support for runtime switching of the default domain, so that can be re-used. Regards, Joerg
On 2021-07-14 11:15, Joerg Roedel wrote: > Hi Robin, > > On Fri, Jul 09, 2021 at 02:56:47PM +0100, Robin Murphy wrote: >> As I mentioned before, conceptually I think this very much belongs in sysfs >> as a user decision. We essentially have 4 levels of "strictness": >> >> 1: DMA domain with bounce pages >> 2: DMA domain >> 3: DMA domain with flush queue >> 4: Identity domain > > Together with reasonable defaults (influenced by compile-time > options) it seems to be a good thing to configure at runtime via > sysfs. > > We already have CONFIG_IOMMU_DEFAULT_PASSTHROUGH, which can probably be > extended to be an option list: > > - CONFIG_IOMMU_DEFAULT_PASSTHROUGH: Trusted devices are identity > mapped > > - CONFIG_IOMMU_DEFAULT_DMA_STRICT: Trusted devices are DMA > mapped with strict flush > behavior on unmap > > - CONFIG_IOMMU_DEFAULT_DMA_LAZY: Trusted devices are DMA mapped > with flush queues for performance Indeed, I got focused on the sysfs angle, but rearranging the Kconfig default that way to match makes a lot of sense, and is another thing which should fall out really easily from my domain type rework, so I'll add that to my branch now before I forget again. > Untrusted devices always get into the DMA domain with bounce pages by > default. > > The defaults can be changed at runtime via sysfs. We already have basic > support for runtime switching of the default domain, so that can be > re-used. As mentioned yesterday, already done! I'm hoping to be able to post the patches next week after some testing :) Cheers, Robin.
On Wed, Jul 14, 2021 at 11:29:08AM +0100, Robin Murphy wrote: > As mentioned yesterday, already done! I'm hoping to be able to post the > patches next week after some testing :) Great, looking forward to your patches :-)
Hi, On Tue, Jul 13, 2021 at 11:07 AM Robin Murphy <robin.murphy@arm.com> wrote: > > On 2021-07-08 15:36, Doug Anderson wrote: > [...] > >> Or document for the users that want performance how to > >> change the setting, so that they can decide. > > > > Pushing this to the users can make sense for a Linux distribution but > > probably less sense for an embedded platform. So I'm happy to make > > some way for a user to override this (like via kernel command line), > > but I also strongly believe there should be a default that users don't > > have to futz with that we think is correct. > > FYI I did make progress on the "punt it to userspace" approach. I'm not > posting it even as an RFC yet because I still need to set up a machine > to try actually testing any of it (it's almost certainly broken > somewhere), but in the end it comes out looking surprisingly not too bad > overall. If you're curious to take a look in the meantime I put it here: > > https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/fq Being able to change this at runtime through sysfs sounds great and it fills all the needs I'm aware of, thanks! In Chrome OS we can just use this with some udev rules and get everything we need. I'm happy to give this a spin when you're ready for extra testing. -Doug
Hi Robin, Doug, On Wed, Jul 14, 2021 at 8:14 AM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Tue, Jul 13, 2021 at 11:07 AM Robin Murphy <robin.murphy@arm.com> wrote: > > > > On 2021-07-08 15:36, Doug Anderson wrote: > > [...] > > >> Or document for the users that want performance how to > > >> change the setting, so that they can decide. > > > > > > Pushing this to the users can make sense for a Linux distribution but > > > probably less sense for an embedded platform. So I'm happy to make > > > some way for a user to override this (like via kernel command line), > > > but I also strongly believe there should be a default that users don't > > > have to futz with that we think is correct. > > > > FYI I did make progress on the "punt it to userspace" approach. I'm not > > posting it even as an RFC yet because I still need to set up a machine > > to try actually testing any of it (it's almost certainly broken > > somewhere), but in the end it comes out looking surprisingly not too bad > > overall. If you're curious to take a look in the meantime I put it here: > > > > https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/fq I was wondering if you got any closer to testing / sending it out? I looked at the patches and am trying to understand, would they also make it possible to convert at runtime, an existing "non-strict" domain (for a particular device) into a "strict" domain leaving the other devices/domains as-is? Please let me know when you think your patches are good to be tested, and I'd also be interested in trying them out. > > Being able to change this at runtime through sysfs sounds great and it > fills all the needs I'm aware of, thanks! In Chrome OS we can just use > this with some udev rules and get everything we need. I still have another (inverse) use case where this does not work: We have an Intel chromebook with the default domain type being non-strict. There is an LTE modem (an internal PCI device which cannot be marked external), which we'd like to be treated as a "Strict" DMA domain. Do I understand it right that using Rob's patches, I could potentially switch the domain to "strict" *after* booting (since we don't use initramfs), but by that time, the driver might have already attached to the modem device (using "non-strict" domain), and thus the damage may have already been done? So perhaps we still need a device property that the firmware could use to indicate "strictness" for certain devices at boot? Thanks, Rajat
Hi Rob, On Mon, Aug 2, 2021 at 5:09 PM Rajat Jain <rajatja@google.com> wrote: > > Hi Robin, Doug, > > On Wed, Jul 14, 2021 at 8:14 AM Doug Anderson <dianders@chromium.org> wrote: > > > > Hi, > > > > On Tue, Jul 13, 2021 at 11:07 AM Robin Murphy <robin.murphy@arm.com> wrote: > > > > > > On 2021-07-08 15:36, Doug Anderson wrote: > > > [...] > > > >> Or document for the users that want performance how to > > > >> change the setting, so that they can decide. > > > > > > > > Pushing this to the users can make sense for a Linux distribution but > > > > probably less sense for an embedded platform. So I'm happy to make > > > > some way for a user to override this (like via kernel command line), > > > > but I also strongly believe there should be a default that users don't > > > > have to futz with that we think is correct. > > > > > > FYI I did make progress on the "punt it to userspace" approach. I'm not > > > posting it even as an RFC yet because I still need to set up a machine > > > to try actually testing any of it (it's almost certainly broken > > > somewhere), but in the end it comes out looking surprisingly not too bad > > > overall. If you're curious to take a look in the meantime I put it here: > > > > > > https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/fq BTW, is there another mirror to this? I (and another colleague) are getting the following error when trying to clone it: rajatja@rajat2:~/rob_iommu$ git clone https://git.gitlab.arm.com/linux-arm/linux-rm.git Cloning into 'linux-rm'... remote: Enumerating objects: 125712, done. remote: Counting objects: 100% (125712/125712), done. remote: Compressing objects: 100% (41203/41203), done. error: RPC failed; curl 18 transfer closed with outstanding read data remaining error: 804 bytes of body are still expected fetch-pack: unexpected disconnect while reading sideband packet fatal: early EOF fatal: fetch-pack: invalid index-pack output rajatja@rajat2:~/rob_iommu$ We've tried both git and https methods. > > I was wondering if you got any closer to testing / sending it out? I > looked at the patches and am trying to understand, would they also > make it possible to convert at runtime, an existing "non-strict" > domain (for a particular device) into a "strict" domain leaving the > other devices/domains as-is? Please let me know when you think your > patches are good to be tested, and I'd also be interested in trying > them out. > > > > > Being able to change this at runtime through sysfs sounds great and it > > fills all the needs I'm aware of, thanks! In Chrome OS we can just use > > this with some udev rules and get everything we need. > > I still have another (inverse) use case where this does not work: > We have an Intel chromebook with the default domain type being > non-strict. There is an LTE modem (an internal PCI device which cannot > be marked external), which we'd like to be treated as a "Strict" DMA > domain. > > Do I understand it right that using Rob's patches, I could potentially > switch the domain to "strict" *after* booting (since we don't use > initramfs), but by that time, the driver might have already attached > to the modem device (using "non-strict" domain), and thus the damage > may have already been done? So perhaps we still need a device property > that the firmware could use to indicate "strictness" for certain > devices at boot? > > Thanks, > Rajat
On 2021-08-03 01:09, Rajat Jain wrote: > Hi Robin, Doug, > > On Wed, Jul 14, 2021 at 8:14 AM Doug Anderson <dianders@chromium.org> wrote: >> >> Hi, >> >> On Tue, Jul 13, 2021 at 11:07 AM Robin Murphy <robin.murphy@arm.com> wrote: >>> >>> On 2021-07-08 15:36, Doug Anderson wrote: >>> [...] >>>>> Or document for the users that want performance how to >>>>> change the setting, so that they can decide. >>>> >>>> Pushing this to the users can make sense for a Linux distribution but >>>> probably less sense for an embedded platform. So I'm happy to make >>>> some way for a user to override this (like via kernel command line), >>>> but I also strongly believe there should be a default that users don't >>>> have to futz with that we think is correct. >>> >>> FYI I did make progress on the "punt it to userspace" approach. I'm not >>> posting it even as an RFC yet because I still need to set up a machine >>> to try actually testing any of it (it's almost certainly broken >>> somewhere), but in the end it comes out looking surprisingly not too bad >>> overall. If you're curious to take a look in the meantime I put it here: >>> >>> https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/fq > > I was wondering if you got any closer to testing / sending it out? I > looked at the patches and am trying to understand, would they also > make it possible to convert at runtime, an existing "non-strict" > domain (for a particular device) into a "strict" domain leaving the > other devices/domains as-is? Please let me know when you think your > patches are good to be tested, and I'd also be interested in trying > them out. Yup, most recently here: https://lore.kernel.org/linux-iommu/cover.1627468308.git.robin.murphy@arm.com/ I'm currently getting v3 ready, so I'll try to remember to add you to the CC list. >> Being able to change this at runtime through sysfs sounds great and it >> fills all the needs I'm aware of, thanks! In Chrome OS we can just use >> this with some udev rules and get everything we need. > > I still have another (inverse) use case where this does not work: > We have an Intel chromebook with the default domain type being > non-strict. There is an LTE modem (an internal PCI device which cannot > be marked external), which we'd like to be treated as a "Strict" DMA > domain. > > Do I understand it right that using Rob's patches, I could potentially > switch the domain to "strict" *after* booting (since we don't use > initramfs), but by that time, the driver might have already attached > to the modem device (using "non-strict" domain), and thus the damage > may have already been done? So perhaps we still need a device property > that the firmware could use to indicate "strictness" for certain > devices at boot? Well, in my view the "external facing" firmware property *should* effectively be the "I don't trust this device not to misbehave" property, but I guess it's a bit too conflated with other aspects of Thunderbolt root ports (at least in the ACPI definition) to abuse in that manner. Ideas off the top of my head would be to flip the default domain type and manually relax all the other performance-sensitive devices instead, or module_blacklist the modem driver to load manually later after tweaking its group. However, if you think it's a sufficiently general concern, maybe a quirk to set pci_dev->untrusted might be worth exploring? It may make sense to drive such a thing from a command-line option rather than a hard-coded list, though, since trust is really down to the individual use-case. [ re gitlab.arm.com, I understand it tends not to like large transfers - some colleagues have reported similar issues pushing large repos as well. I'd suggest cloning the base mainline repo from kernel.org or another reliable source, then fetching my branch into that. I've just tried that on a different machine (outside the work network) and it worked fine) ] Thanks, Robin.
The goal of this patch series is to get better SD/MMC performance on Qualcomm eMMC controllers and in generally nudge us forward on the path of allowing some devices to be in strict mode and others to be in non-strict mode. This patch series doesn't save the world but hopefully at least moves us in the right direction while accomplishing something useful. Specifically: - No attempt is made to touch the PCI subsystem or cleanup the way that it requests strict vs. non-strict. - No fully generic mechanism is come up with that makes it super easy for everyone to be in non-strict mode. This patch conflicts with a few other patch series that are in flight. I've tried to call them out "after the cut" in patches. I assume other in flight patches will land before this one, so I'd expect to send a rebased version when that happens, assuming that this series isn't NAKed into the ground. Changes in v2: - No longer based on changes adding strictness to "struct device" - Updated kernel-parameters docs. - Patch moving check for strictness in arm-smmu new for v2. - Now accomplish the goal by putting rules in the IOMMU driver. - Reworded commit message to clarify things pointed out by Greg. Douglas Anderson (3): iommu: Add per-domain strictness and combine with the global default iommu/arm-smmu: Check for strictness after calling impl->init_context() mmc: sdhci-msm: Request non-strict IOMMU mode .../admin-guide/kernel-parameters.txt | 5 ++- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 19 ++++++++ drivers/iommu/arm/arm-smmu/arm-smmu.c | 6 +-- drivers/iommu/iommu.c | 43 +++++++++++++++---- include/linux/iommu.h | 7 +++ 5 files changed, 67 insertions(+), 13 deletions(-)