[RFC 0/1] tag IOMMU-protected devices in Dom0 fdt

[RFC 0/1] tag IOMMU-protected devices in Dom0 fdt

[Roman Skakun]

From: Roman Skakun <roman_skakun@epam.com>

At the moment, Dom0 can't distinguish which devices are protected by
IOMMU and which are not. In some cases, this can cause swiotlb bounce
buffer use for DMA addresses above 32 bits, which in turn can lead
to poor performance. I started a conversation at [1], where we discussed
addition of a new device tree property to mark IOMMU-protected devices for Dom0.

As a result of negotiation at [1], I would like to present two
patches:

1. The first patch for the hypervisor. It adds a new device property
'xen,behind-iommu' to a relevant device node when the device is
IOMMU-protected.

2. The second patch is a Linux kernel counterpart. It detects the said
property and disables swiotlb for a device.

There is a possible issue: some devices may not be able to use DMA
addresses above 32 bit boundaries, so we can have problems in the
direct DMA mechanism when swiotlb-xen is disabled for a such
device. More generally, this can affect any device which DMA address
range is narrower than CPU one.
 
In this case,
the device DMA address should be no bigger than 32 bit boundaries for
each device that is not using swiotlb-xen.

Several ideas on how to overcome it:
1. Do not use high memory for Dom0.
2. Set DMA 32bit mask for each device if swiotlb is not used for this device.
3. Force balloon driver to allocate buffers only below 4GB.

I will be glad to get any comments or suggestions.

[1] https://lore.kernel.org/xen-devel/AM7PR03MB65936E5D0B25567D1B2FAECA85CC9@AM7PR03MB6593.eurprd03.prod.outlook.com/

Roman Skakun (1):
  xen/arm: set iommu property for IOMMU-protected devices

 xen/arch/arm/domain_build.c | 7 +++++++
 1 file changed, 7 insertions(+)

-- 
2.27.0

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

[Roman Skakun]

From: Roman Skakun <roman_skakun@epam.com>

Xen is not exposing any IOMMU properties to Dom0.
So Dom0 assumes that all it's devices are not protected by IOMMU.

To make Dom0 aware of IOMMU-protected devices, we need to mark
them somehow. With this approach Dom0 Linux kernel will be able
to selectively disable swiotlb-xen fops for them which will remove
unnecessary buffer bounces.

This patch adds mechanism to describe IOMMU-protected devices by
adding `xen,behind-iommu` property to relevant device nodes in
Dom0 device tree.

Signed-off-by: Roman Skakun <roman_skakun@epam.com>
---
 xen/arch/arm/domain_build.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 69fff7fc29..99e2c42b6c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -580,6 +580,13 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
             return res;
     }
 
+    if ( iommu_node && is_iommu_enabled(d) && dt_device_is_protected(node) )
+    {
+        res = fdt_property(kinfo->fdt, "xen,behind-iommu", NULL, 0);
+        if ( res )
+            return res;
+    }
+
     /*
      * Override the property "status" to disable the device when it's
      * marked for passthrough.
-- 
2.27.0

[RFC PATCH] dma-mapping: don't set swiotlb-xen fops for IOMMU-protected devices

[Roman Skakun]

Prior to this patch swiotlb-xen fops are set for all devices when
we are booting as as Dom0 or direct-mapped DomU.

Since Xen now marks devices which are IOMMU-protected by
adding additional property to the relevant device, we can
check for this property and don't set swiotlb-xen fops for
any IOMMU-protected device.

Signed-off-by: Roman Skakun <Roman_Skakun@epam.com>
---
 arch/arm/mm/dma-mapping.c   | 2 +-
 arch/arm/xen/enlighten.c    | 4 ++++
 arch/arm64/mm/dma-mapping.c | 2 +-
 include/xen/xen.h           | 2 ++
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index c4b8df2ad328..c3e5841d871e 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2280,7 +2280,7 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 	set_dma_ops(dev, dma_ops);
 
 #ifdef CONFIG_XEN
-	if (xen_initial_domain())
+	if (xen_initial_domain() && !xen_is_device_protected(dev))
 		dev->dma_ops = &xen_swiotlb_dma_ops;
 #endif
 	dev->archdata.dma_ops_setup = true;
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(void *obj) {
+	struct device *dev = obj;
+	return of_property_read_bool(dev->of_node, "xen,behind-iommu");
+}
 
 /* empty stubs */
 void xen_arch_pre_suspend(void) { }
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 93e87b287556..0af5f7a63124 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -53,7 +53,7 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 		iommu_setup_dma_ops(dev, dma_base, size);
 
 #ifdef CONFIG_XEN
-	if (xen_initial_domain())
+	if (xen_initial_domain() && !xen_is_device_protected(dev))
 		dev->dma_ops = &xen_swiotlb_dma_ops;
 #endif
 }
diff --git a/include/xen/xen.h b/include/xen/xen.h
index 43efba045acc..3725d69de6f9 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -61,4 +61,6 @@ void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages);
 #include <xen/balloon.h>
 #endif
 
+bool xen_is_device_protected(void *obj);
+
 #endif	/* _XEN_XEN_H */
-- 
2.27.0

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

[Oleksandr]

On 04.10.21 12:54, Roman Skakun wrote:

Hi Roman

> From: Roman Skakun <roman_skakun@epam.com>
>
> Xen is not exposing any IOMMU properties to Dom0.
> So Dom0 assumes that all it's devices are not protected by IOMMU.
>
> To make Dom0 aware of IOMMU-protected devices, we need to mark
> them somehow. With this approach Dom0 Linux kernel will be able
> to selectively disable swiotlb-xen fops for them which will remove
> unnecessary buffer bounces.
>
> This patch adds mechanism to describe IOMMU-protected devices by
> adding `xen,behind-iommu` property to relevant device nodes in
> Dom0 device tree.

I think that new property should be documented probably at

docs/misc/arm/device-tree/...

>
> Signed-off-by: Roman Skakun <roman_skakun@epam.com>
> ---
>   xen/arch/arm/domain_build.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 69fff7fc29..99e2c42b6c 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -580,6 +580,13 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
>               return res;
>       }
>   
> +    if ( iommu_node && is_iommu_enabled(d) && dt_device_is_protected(node) )
> +    {
> +        res = fdt_property(kinfo->fdt, "xen,behind-iommu", NULL, 0);
> +        if ( res )
> +            return res;
> +    }
> +
>       /*
>        * Override the property "status" to disable the device when it's
>        * marked for passthrough.

-- 
Regards,

Oleksandr Tyshchenko

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

[Roman Skakun]

Hi Oleksandr,

>> From: Roman Skakun <roman_skakun@epam.com>
>>
>> Xen is not exposing any IOMMU properties to Dom0.
>> So Dom0 assumes that all it's devices are not protected by IOMMU.
>>
>> To make Dom0 aware of IOMMU-protected devices, we need to mark
>> them somehow. With this approach Dom0 Linux kernel will be able
>> to selectively disable swiotlb-xen fops for them which will remove
>> unnecessary buffer bounces.
>>
>> This patch adds mechanism to describe IOMMU-protected devices by
>> adding `xen,behind-iommu` property to relevant device nodes in
>> Dom0 device tree.
>
> I think that new property should be documented probably at
>
> docs/misc/arm/device-tree/...

Yes, make sense.
I will add a description for a new property in the next patch series.
Thanks!

Cheers,
Roman

ср, 6 окт. 2021 г. в 15:45, Oleksandr <olekstysh@gmail.com>:
>
>
> On 04.10.21 12:54, Roman Skakun wrote:
>
> Hi Roman
>
> > From: Roman Skakun <roman_skakun@epam.com>
> >
> > Xen is not exposing any IOMMU properties to Dom0.
> > So Dom0 assumes that all it's devices are not protected by IOMMU.
> >
> > To make Dom0 aware of IOMMU-protected devices, we need to mark
> > them somehow. With this approach Dom0 Linux kernel will be able
> > to selectively disable swiotlb-xen fops for them which will remove
> > unnecessary buffer bounces.
> >
> > This patch adds mechanism to describe IOMMU-protected devices by
> > adding `xen,behind-iommu` property to relevant device nodes in
> > Dom0 device tree.
>
> I think that new property should be documented probably at
>
> docs/misc/arm/device-tree/...
>
> >
> > Signed-off-by: Roman Skakun <roman_skakun@epam.com>
> > ---
> >   xen/arch/arm/domain_build.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 69fff7fc29..99e2c42b6c 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -580,6 +580,13 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
> >               return res;
> >       }
> >
> > +    if ( iommu_node && is_iommu_enabled(d) && dt_device_is_protected(node) )
> > +    {
> > +        res = fdt_property(kinfo->fdt, "xen,behind-iommu", NULL, 0);
> > +        if ( res )
> > +            return res;
> > +    }
> > +
> >       /*
> >        * Override the property "status" to disable the device when it's
> >        * marked for passthrough.
>
> --
> Regards,
>
> Oleksandr Tyshchenko
>


-- 
Best Regards, Roman.

Re: [RFC 0/1] tag IOMMU-protected devices in Dom0 fdt

[Roman Skakun]

Hi,

Would you be so kind to look at my patches, it would help me to
know exactly whether I'm moving correctly and whether I've chosen
the right path?
Thanks!

Best regards,
Roman

пн, 4 окт. 2021 г. в 12:54, Roman Skakun <rm.skakun@gmail.com>:
>
> From: Roman Skakun <roman_skakun@epam.com>
>
> At the moment, Dom0 can't distinguish which devices are protected by
> IOMMU and which are not. In some cases, this can cause swiotlb bounce
> buffer use for DMA addresses above 32 bits, which in turn can lead
> to poor performance. I started a conversation at [1], where we discussed
> addition of a new device tree property to mark IOMMU-protected devices for Dom0.
>
> As a result of negotiation at [1], I would like to present two
> patches:
>
> 1. The first patch for the hypervisor. It adds a new device property
> 'xen,behind-iommu' to a relevant device node when the device is
> IOMMU-protected.
>
> 2. The second patch is a Linux kernel counterpart. It detects the said
> property and disables swiotlb for a device.
>
> There is a possible issue: some devices may not be able to use DMA
> addresses above 32 bit boundaries, so we can have problems in the
> direct DMA mechanism when swiotlb-xen is disabled for a such
> device. More generally, this can affect any device which DMA address
> range is narrower than CPU one.
>
> In this case,
> the device DMA address should be no bigger than 32 bit boundaries for
> each device that is not using swiotlb-xen.
>
> Several ideas on how to overcome it:
> 1. Do not use high memory for Dom0.
> 2. Set DMA 32bit mask for each device if swiotlb is not used for this device.
> 3. Force balloon driver to allocate buffers only below 4GB.
>
> I will be glad to get any comments or suggestions.
>
> [1] https://lore.kernel.org/xen-devel/AM7PR03MB65936E5D0B25567D1B2FAECA85CC9@AM7PR03MB6593.eurprd03.prod.outlook.com/
>
> Roman Skakun (1):
>   xen/arm: set iommu property for IOMMU-protected devices
>
>  xen/arch/arm/domain_build.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> --
> 2.27.0
>


-- 
Best Regards, Roman.

Re: [RFC 0/1] tag IOMMU-protected devices in Dom0 fdt

[Julien Grall]

On 12/10/2021 15:11, Roman Skakun wrote:
> Hi,

Hi Roman,

> Would you be so kind to look at my patches, it would help me to
> know exactly whether I'm moving correctly and whether I've chosen
> the right path?

This is in my queue to review. At the moment, I am prioritizing work 
targeting 4.16. Unfortunately, this RFC was posted after the last 
posting date. So I will review it after the feature freeze.

Best regards,

-- 
Julien Grall

Re: [RFC 0/1] tag IOMMU-protected devices in Dom0 fdt

[Roman Skakun]

Understood.
Thank you!

вт, 12 окт. 2021 г. в 17:20, Julien Grall <julien@xen.org>:
>
>
>
> On 12/10/2021 15:11, Roman Skakun wrote:
> > Hi,
>
> Hi Roman,
>
> > Would you be so kind to look at my patches, it would help me to
> > know exactly whether I'm moving correctly and whether I've chosen
> > the right path?
>
> This is in my queue to review. At the moment, I am prioritizing work
> targeting 4.16. Unfortunately, this RFC was posted after the last
> posting date. So I will review it after the feature freeze.
>
> Best regards,
>
> --
> Julien Grall



-- 
Best Regards, Roman.

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

[Julien Grall]

Hi Roman,

On 04/10/2021 10:54, Roman Skakun wrote:
> From: Roman Skakun <roman_skakun@epam.com>
> 
> Xen is not exposing any IOMMU properties to Dom0.
> So Dom0 assumes that all it's devices are not protected by IOMMU.
> 
> To make Dom0 aware of IOMMU-protected devices, we need to mark
> them somehow. With this approach Dom0 Linux kernel will be able
> to selectively disable swiotlb-xen fops for them which will remove
> unnecessary buffer bounces.
> 
> This patch adds mechanism to describe IOMMU-protected devices by
> adding `xen,behind-iommu` property to relevant device nodes in
> Dom0 device tree.

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?

Cheers,

[1] 
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1392913301-25524-1-git-send-email-julien.grall@linaro.org/

-- 
Julien Grall

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

[Stefano Stabellini]

On Mon, 8 Nov 2021, Julien Grall wrote:
> Hi Roman,
> 
> On 04/10/2021 10:54, Roman Skakun wrote:
> > From: Roman Skakun <roman_skakun@epam.com>
> > 
> > Xen is not exposing any IOMMU properties to Dom0.
> > So Dom0 assumes that all it's devices are not protected by IOMMU.
> > 
> > To make Dom0 aware of IOMMU-protected devices, we need to mark
> > them somehow. With this approach Dom0 Linux kernel will be able
> > to selectively disable swiotlb-xen fops for them which will remove
> > unnecessary buffer bounces.
> > 
> > This patch adds mechanism to describe IOMMU-protected devices by
> > adding `xen,behind-iommu` property to relevant device nodes in
> > Dom0 device tree.
> 
> 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")

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

[Sergiy Kibrik]

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. 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, 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?

  - Sergiy

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

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 3354 bytes --]

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