Re: [PATCH v3 1/2] dt-bindings: arm: xen: document Xen iommu device
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@e60b {
iommus = <_iommu 0x0>;
};
Maybe this driver can be used to solve the deferred problem with some
modifications?
What is your opinion?
Best regards,
Oleksii.
Re: [PATCH v3 1/2] dt-bindings: arm: xen: document Xen iommu device
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@ff {
compatible = "this,device";
xen,iommu = "on";
};
Re: [PATCH v3 1/2] dt-bindings: arm: xen: document Xen iommu device
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
Re: [PATCH v3 1/2] dt-bindings: arm: xen: document Xen iommu device
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.
Re: [PATCH v3 1/2] dt-bindings: arm: xen: document Xen iommu device
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
RE: [PATCH v3 1/2] dt-bindings: arm: xen: document Xen iommu device
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
Re: [PATCH v3 1/2] dt-bindings: arm: xen: document Xen iommu device
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
> > ---
> > 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 = < 0x0>;
> > +};
> > +
> > +[1] Documentation/devicetree/bindings/iommu/iommu.txt
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Re: [PATCH v3 1/2] dt-bindings: arm: xen: document Xen iommu device
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
---
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 = < 0x0>;
+};
+
+[1] Documentation/devicetree/bindings/iommu/iommu.txt
