Re: [PATCH v3 1/2] dt-bindings: arm: xen: document Xen iommu device

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxxx>
> > ---
> >   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@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>