| Message ID | 20210621165230.2.Icfe7cbb2cc86a38dde0ee5ba240e0580a0ec9596@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | iommu: Enable devices to request non-strict DMA, starting with QCom SD/MMC | expand |
On Mon, Jun 21, 2021 at 04:52:44PM -0700, Douglas Anderson wrote: > How to control the "strictness" of an IOMMU is a bit of a mess right > now. As far as I can tell, right now: > * You can set the default to "non-strict" and some devices (right now, > only PCI devices) can request to run in "strict" mode. > * You can set the default to "strict" and no devices in the system are > allowed to run as "non-strict". > > I believe this needs to be improved a bit. Specifically: > * We should be able to default to "strict" mode but let devices that > claim to be fairly low risk request that they be run in "non-strict" > mode. > * We should allow devices outside of PCIe to request "strict" mode if > the system default is "non-strict". > > I believe the correct way to do this is two bits in "struct > device". One allows a device to force things to "strict" mode and the > other allows a device to _request_ "non-strict" mode. The asymmetry > here is on purpose. Generally if anything in the system makes a > request for strictness of something then we want it strict. Thus > drivers can only request (but not force) non-strictness. > > It's expected that the strictness fields can be filled in by the bus > code like in the patch ("PCI: Indicate that we want to force strict > DMA for untrusted devices") or by using the new pre_probe concept > introduced in the patch ("drivers: base: Add the concept of > "pre_probe" to drivers"). > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > include/linux/device.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/include/linux/device.h b/include/linux/device.h > index f1a00040fa53..c1b985e10c47 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -449,6 +449,15 @@ struct dev_links_info { > * and optionall (if the coherent mask is large enough) also > * for dma allocations. This flag is managed by the dma ops > * instance from ->dma_supported. > + * @force_strict_iommu: If set to %true then we should force this device to > + * iommu.strict regardless of the other defaults in the > + * system. Only has an effect if an IOMMU is in place. Why would you ever NOT want to do this? > + * @request_non_strict_iommu: If set to %true and there are no other known > + * reasons to make the iommu.strict for this device, > + * then default to non-strict mode. This implies > + * some belief that the DMA master for this device > + * won't abuse the DMA path to compromise the kernel. > + * Only has an effect if an IOMMU is in place. This feels in contrast to the previous field you just added, how do they both interact? Would an enum work better? thanks, greg k-h
Hi, On Thu, Jun 24, 2021 at 6:37 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Mon, Jun 21, 2021 at 04:52:44PM -0700, Douglas Anderson wrote: > > How to control the "strictness" of an IOMMU is a bit of a mess right > > now. As far as I can tell, right now: > > * You can set the default to "non-strict" and some devices (right now, > > only PCI devices) can request to run in "strict" mode. > > * You can set the default to "strict" and no devices in the system are > > allowed to run as "non-strict". > > > > I believe this needs to be improved a bit. Specifically: > > * We should be able to default to "strict" mode but let devices that > > claim to be fairly low risk request that they be run in "non-strict" > > mode. > > * We should allow devices outside of PCIe to request "strict" mode if > > the system default is "non-strict". > > > > I believe the correct way to do this is two bits in "struct > > device". One allows a device to force things to "strict" mode and the > > other allows a device to _request_ "non-strict" mode. The asymmetry > > here is on purpose. Generally if anything in the system makes a > > request for strictness of something then we want it strict. Thus > > drivers can only request (but not force) non-strictness. > > > > It's expected that the strictness fields can be filled in by the bus > > code like in the patch ("PCI: Indicate that we want to force strict > > DMA for untrusted devices") or by using the new pre_probe concept > > introduced in the patch ("drivers: base: Add the concept of > > "pre_probe" to drivers"). > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > --- > > > > include/linux/device.h | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/include/linux/device.h b/include/linux/device.h > > index f1a00040fa53..c1b985e10c47 100644 > > --- a/include/linux/device.h > > +++ b/include/linux/device.h > > @@ -449,6 +449,15 @@ struct dev_links_info { > > * and optionall (if the coherent mask is large enough) also > > * for dma allocations. This flag is managed by the dma ops > > * instance from ->dma_supported. > > + * @force_strict_iommu: If set to %true then we should force this device to > > + * iommu.strict regardless of the other defaults in the > > + * system. Only has an effect if an IOMMU is in place. > > Why would you ever NOT want to do this? > > > + * @request_non_strict_iommu: If set to %true and there are no other known > > + * reasons to make the iommu.strict for this device, > > + * then default to non-strict mode. This implies > > + * some belief that the DMA master for this device > > + * won't abuse the DMA path to compromise the kernel. > > + * Only has an effect if an IOMMU is in place. > > This feels in contrast to the previous field you just added, how do they > both interact? Would an enum work better? Right that it never makes sense to set both bits so an enum could work better, essentially it would be { dont_care, request_non_strict, force_strict }. In any case the whole idea of doing this at the device level looks like it's not the right way to go anyway, so this patch and the previous pre_probe one are essentially abandoned and I need to send out a v2 with some different approaches. -Doug
diff --git a/include/linux/device.h b/include/linux/device.h index f1a00040fa53..c1b985e10c47 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -449,6 +449,15 @@ struct dev_links_info { * and optionall (if the coherent mask is large enough) also * for dma allocations. This flag is managed by the dma ops * instance from ->dma_supported. + * @force_strict_iommu: If set to %true then we should force this device to + * iommu.strict regardless of the other defaults in the + * system. Only has an effect if an IOMMU is in place. + * @request_non_strict_iommu: If set to %true and there are no other known + * reasons to make the iommu.strict for this device, + * then default to non-strict mode. This implies + * some belief that the DMA master for this device + * won't abuse the DMA path to compromise the kernel. + * Only has an effect if an IOMMU is in place. * * At the lowest level, every device in a Linux system is represented by an * instance of struct device. The device structure contains the information @@ -557,6 +566,8 @@ struct device { #ifdef CONFIG_DMA_OPS_BYPASS bool dma_ops_bypass : 1; #endif + bool force_strict_iommu:1; + bool request_non_strict_iommu:1; }; /**
How to control the "strictness" of an IOMMU is a bit of a mess right now. As far as I can tell, right now: * You can set the default to "non-strict" and some devices (right now, only PCI devices) can request to run in "strict" mode. * You can set the default to "strict" and no devices in the system are allowed to run as "non-strict". I believe this needs to be improved a bit. Specifically: * We should be able to default to "strict" mode but let devices that claim to be fairly low risk request that they be run in "non-strict" mode. * We should allow devices outside of PCIe to request "strict" mode if the system default is "non-strict". I believe the correct way to do this is two bits in "struct device". One allows a device to force things to "strict" mode and the other allows a device to _request_ "non-strict" mode. The asymmetry here is on purpose. Generally if anything in the system makes a request for strictness of something then we want it strict. Thus drivers can only request (but not force) non-strictness. It's expected that the strictness fields can be filled in by the bus code like in the patch ("PCI: Indicate that we want to force strict DMA for untrusted devices") or by using the new pre_probe concept introduced in the patch ("drivers: base: Add the concept of "pre_probe" to drivers"). Signed-off-by: Douglas Anderson <dianders@chromium.org> --- include/linux/device.h | 11 +++++++++++ 1 file changed, 11 insertions(+)