| Message ID | 20220117123251.140867-2-Sergiy_Kibrik@epam.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Avoid using Xen DMA ops when the device is protected by an IOMMU | expand |
On 2022-01-17 12:32, Sergiy Kibrik wrote: > In IOMMU-capable system hypervisor usually takes over IOMMU control. > Generally guest domains don't need to care about IOMMU management and take any > extra actions. Yet in some cases a knowledge about which device is protected > may be useful for privileged domain. > > In compliance with iommu bindings this can be achieved with device-level > iommus property specified with dummy Xen iommu device. This could break Linux guests, since depending on the deferred probe timeout setting it could lead to drivers never probing because the "IOMMU" never becomes available. Unless you intend to expose actual paravirtualised IOMMU translation functionality to guests (in which case virtio-iommu would be highly preferable anyway), I don't think this is the right approach. If there's no better alternative to using DT to communicate Xen-specific policy, then at least it should logically be via a Xen-specific DT property. Thanks, Robin. > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> > --- > Documentation/devicetree/bindings/arm/xen.txt | 26 +++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt > index db5c56db30ec..98efa95c0d1b 100644 > --- a/Documentation/devicetree/bindings/arm/xen.txt > +++ b/Documentation/devicetree/bindings/arm/xen.txt > @@ -58,3 +58,29 @@ Documentation/arm/uefi.rst, which are provided by the regular UEFI stub. However > they differ because they are provided by the Xen hypervisor, together with a set > of UEFI runtime services implemented via hypercalls, see > http://xenbits.xen.org/docs/unstable/hypercall/x86_64/include,public,platform.h.html. > + > +* XEN IOMMU device > + > +In compliance with iommu bindings Xen virtual IOMMU device node represents > +hypervisor-managed IOMMU [1]. Platform devices specified as IOMMU masters of > +this xen-iommu device are protected by hypervisor-managed platform IOMMU. > + > +Required properties: > + > +- compatible: Should be "xen,iommu-el2-v1" > +- #iommu-cells: must be 0 > + > +Example: > + > +xen-iommu { > + compatible = "xen,iommu-el2-v1"; > + #iommu-cells = <0>; > +}; > + > +video@fe001000 { > + ... > + /* this platform device is IOMMU-protected by hypervisor */ > + iommus = <&xen-iommu 0x0>; > +}; > + > +[1] Documentation/devicetree/bindings/iommu/iommu.txt
On Mon, 17 Jan 2022, Robin Murphy wrote: > On 2022-01-17 12:32, Sergiy Kibrik wrote: > > In IOMMU-capable system hypervisor usually takes over IOMMU control. > > Generally guest domains don't need to care about IOMMU management and take > > any > > extra actions. Yet in some cases a knowledge about which device is protected > > may be useful for privileged domain. > > > > In compliance with iommu bindings this can be achieved with device-level > > iommus property specified with dummy Xen iommu device. > > This could break Linux guests, since depending on the deferred probe timeout > setting it could lead to drivers never probing because the "IOMMU" never > becomes available. > > Unless you intend to expose actual paravirtualised IOMMU translation > functionality to guests (in which case virtio-iommu would be highly preferable > anyway), I don't think this is the right approach. If there's no better > alternative to using DT to communicate Xen-specific policy, then at least it > should logically be via a Xen-specific DT property. Hi Robin, Let me explain why this is useful and how it differs from something like virtio-iommu. When Linux is running as dom0 it uses a swiotlb-based driver to ensure that DMA operations involving foreign (foreign == owned by other VMs) pages can work successfully. The driver is drivers/xen/swiotlb-xen.c. For this discussion the interesting thing about swiotlb-xen is that it is complex, it can be slow, and it is not actually always needed. You can imagine that somebody might want to disable swiotlb-xen whenever possible. Specifically, if a DMA-capable device is behind an IOMMU, then there is no need for Linux to use swiotlb-xen. The problem is that Linux doesn't know when a device is protected by an IOMMU because the IOMMU is used by Xen and not exposed to Linux. This bindings is a way for Xen to provide information to Linux, so that Linux knows when to safely skip swiotlb-xen. It is telling Linux that the device is protected by an IOMMU, even if Linux is not expected to use it or do anything with it. The initial idea was actually to write a Xen-specific DT property for this. Then a member of the community suggested to re-use the IOMMU bindings given that after all this is an IOMMU; the only thing special about it is that the Linux driver is not supposed to do anything with it. In my mind there should actually be a driver for "xen,iommu-el2-v1" in Linux, it is just that the driver would "return 0" without doing anything. (Note that the current patch series doesn't do this as it turned out not to be currently necessary.) This is a long way to say that: 1) from my point of view it is also OK to introduce a Xen-specific property for this if you think it is best (it doesn't look like it would make the code more complex either) 2) at the same time I think that reusing the IOMMU bindings seems fit for purpose In regards to the "deferred probe timeout", I take it wouldn't be an issue if we use a Xen-specific property instead? Also, would the presence of an empty IOMMU driver for it (one that just return success from the probing function without doing anything) solve the problem too? Thanks for taking a look. Cheers, Stefano > > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> > > --- > > Documentation/devicetree/bindings/arm/xen.txt | 26 +++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/arm/xen.txt > > b/Documentation/devicetree/bindings/arm/xen.txt > > index db5c56db30ec..98efa95c0d1b 100644 > > --- a/Documentation/devicetree/bindings/arm/xen.txt > > +++ b/Documentation/devicetree/bindings/arm/xen.txt > > @@ -58,3 +58,29 @@ Documentation/arm/uefi.rst, which are provided by the > > regular UEFI stub. However > > they differ because they are provided by the Xen hypervisor, together with > > a set > > of UEFI runtime services implemented via hypercalls, see > > http://xenbits.xen.org/docs/unstable/hypercall/x86_64/include,public,platform.h.html. > > + > > +* XEN IOMMU device > > + > > +In compliance with iommu bindings Xen virtual IOMMU device node represents > > +hypervisor-managed IOMMU [1]. Platform devices specified as IOMMU masters > > of > > +this xen-iommu device are protected by hypervisor-managed platform IOMMU. > > + > > +Required properties: > > + > > +- compatible: Should be "xen,iommu-el2-v1" > > +- #iommu-cells: must be 0 > > + > > +Example: > > + > > +xen-iommu { > > + compatible = "xen,iommu-el2-v1"; > > + #iommu-cells = <0>; > > +}; > > + > > +video@fe001000 { > > + ... > > + /* this platform device is IOMMU-protected by hypervisor */ > > + iommus = <&xen-iommu 0x0>; > > +}; > > + > > +[1] Documentation/devicetree/bindings/iommu/iommu.txt > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
Hi Robin, > > This could break Linux guests, since depending on the deferred probe > timeout setting it could lead to drivers never probing because the "IOMMU" > never becomes available. > I've noticed no deferred probe timeouts when booting with this patch. Could you please explain more on how this would break guests? Thank you! -- Sergiy
On 2022-01-26 15:09, Sergiy Kibrik wrote: > Hi Robin, > >> >> This could break Linux guests, since depending on the deferred probe >> timeout setting it could lead to drivers never probing because the "IOMMU" >> never becomes available. >> > > I've noticed no deferred probe timeouts when booting with this patch. Could you please explain more on how this would break guests? Right now I think it would actually require command-line intervention, e.g. "fw_devlink=on" or "deferred_probe_timeout=3600" (with modules enabled for the latter to take full effect), but I'm wary of the potential for future config options to control those behaviours by default. Robin. > Thank you! > > -- Sergiy
On Wed, 26 Jan 2022, Robin Murphy wrote: > On 2022-01-26 15:09, Sergiy Kibrik wrote: > > Hi Robin, > > > > > > > > This could break Linux guests, since depending on the deferred probe > > > timeout setting it could lead to drivers never probing because the "IOMMU" > > > never becomes available. > > > > > > > I've noticed no deferred probe timeouts when booting with this patch. Could > > you please explain more on how this would break guests? > > Right now I think it would actually require command-line intervention, e.g. > "fw_devlink=on" or "deferred_probe_timeout=3600" (with modules enabled for the > latter to take full effect), but I'm wary of the potential for future config > options to control those behaviours by default. If deferred_probe_timeout=3600 was specified, we would just need an IOMMU driver in Linux for the "xen,iommu-el2-v1" node to solve the problem, right? I guess I am trying to say that it wouldn't be a device tree interface problem but rather a Linux implementation discussion.
On Wed, Jan 26, 2022 at 10:56:39AM -0800, Stefano Stabellini wrote: > On Wed, 26 Jan 2022, Robin Murphy wrote: > > On 2022-01-26 15:09, Sergiy Kibrik wrote: > > > Hi Robin, > > > > > > > > > > > This could break Linux guests, since depending on the deferred probe > > > > timeout setting it could lead to drivers never probing because the "IOMMU" > > > > never becomes available. > > > > > > > > > > I've noticed no deferred probe timeouts when booting with this patch. Could > > > you please explain more on how this would break guests? > > > > Right now I think it would actually require command-line intervention, e.g. > > "fw_devlink=on" or "deferred_probe_timeout=3600" (with modules enabled for the > > latter to take full effect), but I'm wary of the potential for future config > > options to control those behaviours by default. fw_devlink=on is now the default (for at least a couple of cycles). > > If deferred_probe_timeout=3600 was specified, we would just need an > IOMMU driver in Linux for the "xen,iommu-el2-v1" node to solve the > problem, right? I guess I am trying to say that it wouldn't be a device > tree interface problem but rather a Linux implementation discussion. You would have to add that IOMMU driver to old, existing kernels if you want compatibility with a new DT. Otherwise, that kernel would stop booting with a new DT. Rob
On Fri, 4 Feb 2022, Rob Herring wrote: > On Wed, Jan 26, 2022 at 10:56:39AM -0800, Stefano Stabellini wrote: > > On Wed, 26 Jan 2022, Robin Murphy wrote: > > > On 2022-01-26 15:09, Sergiy Kibrik wrote: > > > > Hi Robin, > > > > > > > > > > > > > > This could break Linux guests, since depending on the deferred probe > > > > > timeout setting it could lead to drivers never probing because the "IOMMU" > > > > > never becomes available. > > > > > > > > > > > > > I've noticed no deferred probe timeouts when booting with this patch. Could > > > > you please explain more on how this would break guests? > > > > > > Right now I think it would actually require command-line intervention, e.g. > > > "fw_devlink=on" or "deferred_probe_timeout=3600" (with modules enabled for the > > > latter to take full effect), but I'm wary of the potential for future config > > > options to control those behaviours by default. > > fw_devlink=on is now the default (for at least a couple of cycles). > > > > > If deferred_probe_timeout=3600 was specified, we would just need an > > IOMMU driver in Linux for the "xen,iommu-el2-v1" node to solve the > > problem, right? I guess I am trying to say that it wouldn't be a device > > tree interface problem but rather a Linux implementation discussion. > > You would have to add that IOMMU driver to old, existing kernels if you > want compatibility with a new DT. Otherwise, that kernel would stop > booting with a new DT. The tiny "xen,iommu-el2-v1" driver could be backported to the stable trees, I would imagine. Otherwise, do you have another suggestion? It looks like fw_devlink=on applies to supplier/consumer interfaces. If that is the problem, then maybe we should avoid supplier/consumer interfaces altogether. Instead, we could add a new Xen specific property, e.g.: device@ff0000 { compatible = "this,device"; xen,iommu = "on"; };
Hi Stefano, > The tiny "xen,iommu-el2-v1" driver could be backported to the stable > trees, I would imagine. Otherwise, do you have another suggestion? There is stub IOMMU driver already merged to the Linux Kernel: Commit 1ca55d50e50c74747a7b8846dac306fbe5ac4cf5 ("xen/grant-dma-iommu: Introduce stub IOMMU driver" added by Oleksandr Tyshchenko. I was able to use it as an empty IOMMU driver on my test setup: Made the following device-tree changes: xen_iommu: xen-iommu { compatible = "xen,grant-dma"; iommu-cells = <0>; }; i2c@e60b0000 { iommus = <&xen_iommu 0x0>; }; Maybe this driver can be used to solve the deferred problem with some modifications? What is your opinion? Best regards, Oleksii.
diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt index db5c56db30ec..98efa95c0d1b 100644 --- a/Documentation/devicetree/bindings/arm/xen.txt +++ b/Documentation/devicetree/bindings/arm/xen.txt @@ -58,3 +58,29 @@ Documentation/arm/uefi.rst, which are provided by the regular UEFI stub. However they differ because they are provided by the Xen hypervisor, together with a set of UEFI runtime services implemented via hypercalls, see http://xenbits.xen.org/docs/unstable/hypercall/x86_64/include,public,platform.h.html. + +* XEN IOMMU device + +In compliance with iommu bindings Xen virtual IOMMU device node represents +hypervisor-managed IOMMU [1]. Platform devices specified as IOMMU masters of +this xen-iommu device are protected by hypervisor-managed platform IOMMU. + +Required properties: + +- compatible: Should be "xen,iommu-el2-v1" +- #iommu-cells: must be 0 + +Example: + +xen-iommu { + compatible = "xen,iommu-el2-v1"; + #iommu-cells = <0>; +}; + +video@fe001000 { + ... + /* this platform device is IOMMU-protected by hypervisor */ + iommus = <&xen-iommu 0x0>; +}; + +[1] Documentation/devicetree/bindings/iommu/iommu.txt
In IOMMU-capable system hypervisor usually takes over IOMMU control. Generally guest domains don't need to care about IOMMU management and take any extra actions. Yet in some cases a knowledge about which device is protected may be useful for privileged domain. In compliance with iommu bindings this can be achieved with device-level iommus property specified with dummy Xen iommu device. Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> --- Documentation/devicetree/bindings/arm/xen.txt | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+)