Re: [RFC 1/1] xen/arm: set iommu property for IOMMU-protected devices

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 7 Dec 2021, Sergiy Kibrik wrote:
> hi Stefano, Julien,
> 
> On 11/10/21 11:12 пп, Stefano Stabellini wrote:
> > On Mon, 8 Nov 2021, Julien Grall wrote:
> [..]
> > > A few years ago, I attempted to disable the swiotlb when Xen configured
> > > the
> > > IOMMU for the device (see [1]). Did you have a chance to go through the
> > > thread? In particular, I think Ian Campbell suggestion about creating an
> > > IOMMU
> > > binding is quite interesting.
> > > 
> > > Stefano, what do you think?
> > 
> > Yes I think it is a good idea. In fact, thinking more about it, it is
> > really the best option. Regardless of the implementation (swiotlb or
> > whatever) the device tree description is likely to look similar to the
> > description of an IOMMU because it is the common pattern shared by all
> > controllers (reset, power, clocks, etc.) so it makes sense to re-use it.
> > 
> > - there is one controller node (the "IOMMU")
> > - there is one property under each device node that is protected,
> >    pointing to the controller with a phandle and optional parameters (in
> >    the case of IOMMUs it is called "iommus")
> > 
> 
> Code in arch_setup_dma_ops() always forces swiotlb for dom0 regardless of any
> prior IOMMU configuration for the device.

Yes. The only exception is cases where XENFEAT_not_direct_mapped is set.


> So if we are to re-use IOMMU bindings and implement kind of dummy
> iommu (that merely does direct allocation and mapping) we'll have to
> check whether device is protected anyway

Yes, the Linux-side implementation wouldn't change very much compared to
what you had in mind, it is just that the device tree part would be
cleaner and more spec compliant.

We would still end up with a property for each to device to specify that
is protected by the IOMMU, it is just that instead of "xen,behind-iommu"
it would be called "iommus" and it would be pointing to a special "fake"
Xen IOMMU node. E.g.:


xen-iommu {
    compatible = "xen,iommu-el2-v1";
    status = "okay";
    #iommu-cells = <0x0>;
};


device@ff00000 {
    compatible = "vendor,device";
    reg = <0xff00000 0x1000>;
    iommus = <&xen-iommu>;
};


I can imagine it could a be problem to try to comply to the internal
iommu API in Linux. The Linux driver for xen-iommu could be tiny. It
doesn't have to implement the Linux iommu API because struct iommu_ops
is massive.

Linux would still have to check for each device if it is protected by
the iommu, but the "iommus" property is parsed automatically by
drivers/of/property.c:of_link_property. It should make the Linux side
easier to implement. of_link_property creates a fwnode "link" between
device@ff00000 and xen-iommu automatically for you, then I think you can
just call fwnode_find_reference to check if device@ff00000 is behind an
IOMMU.



> e.g.:
> 
>   diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>   index 49f566ad9acb..6ddef3233095 100644
>   --- a/arch/arm/xen/enlighten.c
>   +++ b/arch/arm/xen/enlighten.c
>   @@ -425,6 +425,10 @@ static int __init xen_pm_init(void)
>    }
>    late_initcall(xen_pm_init);
> 
>   +bool xen_is_device_protected(struct device *dev) {
>   +   return dev->dma_ops == &dummy_xen_iommu_ops;
>   +}
> 
>    /* empty stubs */
>    void xen_arch_pre_suspend(void) { }
> 
> 
> Have I got it right?

I don't think I understand this example