| Message ID | 20220111112611.90508-1-Sergiy_Kibrik@epam.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN,v3] xen/arm: introduce dummy iommu node for dom0 | expand |
On Tue, 11 Jan 2022, Sergiy Kibrik wrote: > Currently no IOMMU properties are exposed to dom0, thus kernel by default > assumes no protection and enables swiotlb-xen, which leads to costly and > unnecessary buffers bouncing. > > To let kernel know which device is behing IOMMU and hence needs no swiotlb > services we introduce dummy xen-iommu node in FDT and link protected device > nodes to it, using here device tree iommu bindings. > > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> I think the patch looks good. I have no further comments on the code: Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Before committing it to the Xen tree, I'd like to wait that the Linux side, especially the change to Documentation/devicetree/bindings/arm/xen.txt, is acked. > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien@xen.org> > Cc: Oleksandr Tyshchenko <olekstysh@gmail.com> > Cc: Andrii Anisov <Andrii_Anisov@epam.com> > > > Changelog: > > v3: rebased over staging & remove redundand phandle_iommu attribute, discussion: > https://lists.xenproject.org/archives/html/xen-devel/2021-12/msg01753.html > > v2: re-use common iommu dt bindings to let guests know which devices are protected: > https://lists.xenproject.org/archives/html/xen-devel/2021-10/msg00073.html > > xen/arch/arm/domain_build.c | 42 +++++++++++++++++++++++++++ > xen/include/public/device_tree_defs.h | 1 + > 2 files changed, 43 insertions(+) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 6931c022a2..b82ba72fac 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -845,6 +845,12 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo, > } > } > > + if ( iommu_node && is_iommu_enabled(d) && dt_device_is_protected(node) ) > + { > + res = fdt_property_cell(kinfo->fdt, "iommus", GUEST_PHANDLE_IOMMU); > + if ( res ) > + return res; > + } > return 0; > } > > @@ -1479,6 +1485,38 @@ static int __init make_cpus_node(const struct domain *d, void *fdt) > return res; > } > > +static int __init make_iommu_node(const struct domain *d, > + const struct kernel_info *kinfo) > +{ > + const char compat[] = "xen,iommu-el2-v1"; > + int res; > + > + if ( !is_iommu_enabled(d) ) > + return 0; > + > + dt_dprintk("Create iommu node\n"); > + > + res = fdt_begin_node(kinfo->fdt, "xen-iommu"); > + if ( res ) > + return res; > + > + res = fdt_property(kinfo->fdt, "compatible", compat, sizeof(compat)); > + if ( res ) > + return res; > + > + res = fdt_property_cell(kinfo->fdt, "#iommu-cells", 0); > + if ( res ) > + return res; > + > + res = fdt_property_cell(kinfo->fdt, "phandle", GUEST_PHANDLE_IOMMU); > + > + res = fdt_end_node(kinfo->fdt); > + if ( res ) > + return res; > + > + return res; > +} > + > static int __init make_gic_node(const struct domain *d, void *fdt, > const struct dt_device_node *node) > { > @@ -2127,6 +2165,10 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, > if ( res ) > return res; > > + res = make_iommu_node(d, kinfo); > + if ( res ) > + return res; > + > res = make_memory_node(d, kinfo->fdt, addrcells, sizecells, &kinfo->mem); > if ( res ) > return res; > diff --git a/xen/include/public/device_tree_defs.h b/xen/include/public/device_tree_defs.h > index 209d43de3f..df58944bd0 100644 > --- a/xen/include/public/device_tree_defs.h > +++ b/xen/include/public/device_tree_defs.h > @@ -7,6 +7,7 @@ > * onwards. Reserve a high value for the GIC phandle. > */ > #define GUEST_PHANDLE_GIC (65000) > +#define GUEST_PHANDLE_IOMMU (GUEST_PHANDLE_GIC + 1) > > #define GUEST_ROOT_ADDRESS_CELLS 2 > #define GUEST_ROOT_SIZE_CELLS 2 > -- > 2.25.1 >
Hi, > On 11 Jan 2022, at 11:26 am, Sergiy Kibrik <Sergiy_Kibrik@epam.com> wrote: > > Currently no IOMMU properties are exposed to dom0, thus kernel by default > assumes no protection and enables swiotlb-xen, which leads to costly and > unnecessary buffers bouncing. > > To let kernel know which device is behing IOMMU and hence needs no swiotlb > services we introduce dummy xen-iommu node in FDT and link protected device > nodes to it, using here device tree iommu bindings. > > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> Reviewed-by: Rahul Singh <rahul.singh@arm.com> Regards, Rahul > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien@xen.org> > Cc: Oleksandr Tyshchenko <olekstysh@gmail.com> > Cc: Andrii Anisov <Andrii_Anisov@epam.com> > > > Changelog: > > v3: rebased over staging & remove redundand phandle_iommu attribute, discussion: > https://lists.xenproject.org/archives/html/xen-devel/2021-12/msg01753.html > > v2: re-use common iommu dt bindings to let guests know which devices are protected: > https://lists.xenproject.org/archives/html/xen-devel/2021-10/msg00073.html > > xen/arch/arm/domain_build.c | 42 +++++++++++++++++++++++++++ > xen/include/public/device_tree_defs.h | 1 + > 2 files changed, 43 insertions(+) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 6931c022a2..b82ba72fac 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -845,6 +845,12 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo, > } > } > > + if ( iommu_node && is_iommu_enabled(d) && dt_device_is_protected(node) ) > + { > + res = fdt_property_cell(kinfo->fdt, "iommus", GUEST_PHANDLE_IOMMU); > + if ( res ) > + return res; > + } > return 0; > } > > @@ -1479,6 +1485,38 @@ static int __init make_cpus_node(const struct domain *d, void *fdt) > return res; > } > > +static int __init make_iommu_node(const struct domain *d, > + const struct kernel_info *kinfo) > +{ > + const char compat[] = "xen,iommu-el2-v1"; > + int res; > + > + if ( !is_iommu_enabled(d) ) > + return 0; > + > + dt_dprintk("Create iommu node\n"); > + > + res = fdt_begin_node(kinfo->fdt, "xen-iommu"); > + if ( res ) > + return res; > + > + res = fdt_property(kinfo->fdt, "compatible", compat, sizeof(compat)); > + if ( res ) > + return res; > + > + res = fdt_property_cell(kinfo->fdt, "#iommu-cells", 0); > + if ( res ) > + return res; > + > + res = fdt_property_cell(kinfo->fdt, "phandle", GUEST_PHANDLE_IOMMU); > + > + res = fdt_end_node(kinfo->fdt); > + if ( res ) > + return res; > + > + return res; > +} > + > static int __init make_gic_node(const struct domain *d, void *fdt, > const struct dt_device_node *node) > { > @@ -2127,6 +2165,10 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, > if ( res ) > return res; > > + res = make_iommu_node(d, kinfo); > + if ( res ) > + return res; > + > res = make_memory_node(d, kinfo->fdt, addrcells, sizecells, &kinfo->mem); > if ( res ) > return res; > diff --git a/xen/include/public/device_tree_defs.h b/xen/include/public/device_tree_defs.h > index 209d43de3f..df58944bd0 100644 > --- a/xen/include/public/device_tree_defs.h > +++ b/xen/include/public/device_tree_defs.h > @@ -7,6 +7,7 @@ > * onwards. Reserve a high value for the GIC phandle. > */ > #define GUEST_PHANDLE_GIC (65000) > +#define GUEST_PHANDLE_IOMMU (GUEST_PHANDLE_GIC + 1) > > #define GUEST_ROOT_ADDRESS_CELLS 2 > #define GUEST_ROOT_SIZE_CELLS 2 > -- > 2.25.1 > >
> -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Rahul Singh > Sent: 2022年1月14日 16:22 > To: Sergiy Kibrik <Sergiy_Kibrik@epam.com> > Cc: xen-devel <xen-devel@lists.xenproject.org>; Stefano Stabellini > <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Oleksandr > Tyshchenko <olekstysh@gmail.com>; Andrii Anisov > <Andrii_Anisov@epam.com> > Subject: Re: [XEN PATCH v3] xen/arm: introduce dummy iommu node for > dom0 > > Hi, > > > On 11 Jan 2022, at 11:26 am, Sergiy Kibrik <Sergiy_Kibrik@epam.com> > wrote: > > > > Currently no IOMMU properties are exposed to dom0, thus kernel by > default > > assumes no protection and enables swiotlb-xen, which leads to costly and > > unnecessary buffers bouncing. > > > > To let kernel know which device is behing IOMMU and hence needs no [Jiamei Xie] behing? Typo Best wishes Jiamei Xie > swiotlb > > services we introduce dummy xen-iommu node in FDT and link protected > device > > nodes to it, using here device tree iommu bindings. > > > > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> > > Reviewed-by: Rahul Singh <rahul.singh@arm.com> > > Regards, > Rahul > > --- > > Cc: Stefano Stabellini <sstabellini@kernel.org> > > Cc: Julien Grall <julien@xen.org> > > Cc: Oleksandr Tyshchenko <olekstysh@gmail.com> > > Cc: Andrii Anisov <Andrii_Anisov@epam.com> > > > > > > Changelog: > > > > v3: rebased over staging & remove redundand phandle_iommu attribute, > discussion: > > https://lists.xenproject.org/archives/html/xen-devel/2021- > 12/msg01753.html > > > > v2: re-use common iommu dt bindings to let guests know which devices > are protected: > > https://lists.xenproject.org/archives/html/xen-devel/2021- > 10/msg00073.html > > > > xen/arch/arm/domain_build.c | 42 > +++++++++++++++++++++++++++ > > xen/include/public/device_tree_defs.h | 1 + > > 2 files changed, 43 insertions(+) > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > index 6931c022a2..b82ba72fac 100644 > > --- a/xen/arch/arm/domain_build.c > > +++ b/xen/arch/arm/domain_build.c > > @@ -845,6 +845,12 @@ static int __init write_properties(struct domain *d, > struct kernel_info *kinfo, > > } > > } > > > > + if ( iommu_node && is_iommu_enabled(d) && > dt_device_is_protected(node) ) > > + { > > + res = fdt_property_cell(kinfo->fdt, "iommus", > GUEST_PHANDLE_IOMMU); > > + if ( res ) > > + return res; > > + } > > return 0; > > } > > > > @@ -1479,6 +1485,38 @@ static int __init make_cpus_node(const struct > domain *d, void *fdt) > > return res; > > } > > > > +static int __init make_iommu_node(const struct domain *d, > > + const struct kernel_info *kinfo) > > +{ > > + const char compat[] = "xen,iommu-el2-v1"; > > + int res; > > + > > + if ( !is_iommu_enabled(d) ) > > + return 0; > > + > > + dt_dprintk("Create iommu node\n"); > > + > > + res = fdt_begin_node(kinfo->fdt, "xen-iommu"); > > + if ( res ) > > + return res; > > + > > + res = fdt_property(kinfo->fdt, "compatible", compat, sizeof(compat)); > > + if ( res ) > > + return res; > > + > > + res = fdt_property_cell(kinfo->fdt, "#iommu-cells", 0); > > + if ( res ) > > + return res; > > + > > + res = fdt_property_cell(kinfo->fdt, "phandle", > GUEST_PHANDLE_IOMMU); > > + > > + res = fdt_end_node(kinfo->fdt); > > + if ( res ) > > + return res; > > + > > + return res; > > +} > > + > > static int __init make_gic_node(const struct domain *d, void *fdt, > > const struct dt_device_node *node) > > { > > @@ -2127,6 +2165,10 @@ static int __init handle_node(struct domain *d, > struct kernel_info *kinfo, > > if ( res ) > > return res; > > > > + res = make_iommu_node(d, kinfo); > > + if ( res ) > > + return res; > > + > > res = make_memory_node(d, kinfo->fdt, addrcells, sizecells, &kinfo- > >mem); > > if ( res ) > > return res; > > diff --git a/xen/include/public/device_tree_defs.h > b/xen/include/public/device_tree_defs.h > > index 209d43de3f..df58944bd0 100644 > > --- a/xen/include/public/device_tree_defs.h > > +++ b/xen/include/public/device_tree_defs.h > > @@ -7,6 +7,7 @@ > > * onwards. Reserve a high value for the GIC phandle. > > */ > > #define GUEST_PHANDLE_GIC (65000) > > +#define GUEST_PHANDLE_IOMMU (GUEST_PHANDLE_GIC + 1) > > > > #define GUEST_ROOT_ADDRESS_CELLS 2 > > #define GUEST_ROOT_SIZE_CELLS 2 > > -- > > 2.25.1 > > > > >
> > > Currently no IOMMU properties are exposed to dom0, thus kernel by > > default > > > assumes no protection and enables swiotlb-xen, which leads to costly > > > and unnecessary buffers bouncing. > > > > > > To let kernel know which device is behing IOMMU and hence needs no > [Jiamei Xie] > behing? Typo > Oops, typo indeed. Thank you! - Sergiy
Hi, On 11/01/2022 11:26, Sergiy Kibrik wrote: > Currently no IOMMU properties are exposed to dom0, thus kernel by default > assumes no protection and enables swiotlb-xen, which leads to costly and > unnecessary buffers bouncing. > > To let kernel know which device is behing IOMMU and hence needs no swiotlb > services we introduce dummy xen-iommu node in FDT and link protected device > nodes to it, using here device tree iommu bindings. > > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien@xen.org> > Cc: Oleksandr Tyshchenko <olekstysh@gmail.com> > Cc: Andrii Anisov <Andrii_Anisov@epam.com> > > > Changelog: > > v3: rebased over staging & remove redundand phandle_iommu attribute, discussion: > https://lists.xenproject.org/archives/html/xen-devel/2021-12/msg01753.html > > v2: re-use common iommu dt bindings to let guests know which devices are protected: > https://lists.xenproject.org/archives/html/xen-devel/2021-10/msg00073.html > > xen/arch/arm/domain_build.c | 42 +++++++++++++++++++++++++++ > xen/include/public/device_tree_defs.h | 1 + > 2 files changed, 43 insertions(+) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 6931c022a2..b82ba72fac 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -845,6 +845,12 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo, > } > } > > + if ( iommu_node && is_iommu_enabled(d) && dt_device_is_protected(node) ) I think it should be sufficient to check dt_device_is_protected() because it is set it means that device behind an IOMMU known by Xen. So iommu_node will always be valid. Furthermore, you can't assign to dom0 a device that was protected with enabling the IOMMU for the domain. > + { > + res = fdt_property_cell(kinfo->fdt, "iommus", GUEST_PHANDLE_IOMMU); > + if ( res ) > + return res; > + } > return 0; > } > > @@ -1479,6 +1485,38 @@ static int __init make_cpus_node(const struct domain *d, void *fdt) > return res; > } > > +static int __init make_iommu_node(const struct domain *d, > + const struct kernel_info *kinfo) > +{ > + const char compat[] = "xen,iommu-el2-v1"; > + int res; > + > + if ( !is_iommu_enabled(d) ) > + return 0; > + > + dt_dprintk("Create iommu node\n"); > + > + res = fdt_begin_node(kinfo->fdt, "xen-iommu"); > + if ( res ) > + return res; > + > + res = fdt_property(kinfo->fdt, "compatible", compat, sizeof(compat)); > + if ( res ) > + return res; > + > + res = fdt_property_cell(kinfo->fdt, "#iommu-cells", 0); > + if ( res ) > + return res; > + > + res = fdt_property_cell(kinfo->fdt, "phandle", GUEST_PHANDLE_IOMMU); Please don't hardocode the phandle for the IOMMU. Instead we should use one for an IOMMU that is used by Xen. This will reduce the risk to use a phandle that could be possibly used in the host Device-Tree. Cheers,
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 6931c022a2..b82ba72fac 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -845,6 +845,12 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo, } } + if ( iommu_node && is_iommu_enabled(d) && dt_device_is_protected(node) ) + { + res = fdt_property_cell(kinfo->fdt, "iommus", GUEST_PHANDLE_IOMMU); + if ( res ) + return res; + } return 0; } @@ -1479,6 +1485,38 @@ static int __init make_cpus_node(const struct domain *d, void *fdt) return res; } +static int __init make_iommu_node(const struct domain *d, + const struct kernel_info *kinfo) +{ + const char compat[] = "xen,iommu-el2-v1"; + int res; + + if ( !is_iommu_enabled(d) ) + return 0; + + dt_dprintk("Create iommu node\n"); + + res = fdt_begin_node(kinfo->fdt, "xen-iommu"); + if ( res ) + return res; + + res = fdt_property(kinfo->fdt, "compatible", compat, sizeof(compat)); + if ( res ) + return res; + + res = fdt_property_cell(kinfo->fdt, "#iommu-cells", 0); + if ( res ) + return res; + + res = fdt_property_cell(kinfo->fdt, "phandle", GUEST_PHANDLE_IOMMU); + + res = fdt_end_node(kinfo->fdt); + if ( res ) + return res; + + return res; +} + static int __init make_gic_node(const struct domain *d, void *fdt, const struct dt_device_node *node) { @@ -2127,6 +2165,10 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, if ( res ) return res; + res = make_iommu_node(d, kinfo); + if ( res ) + return res; + res = make_memory_node(d, kinfo->fdt, addrcells, sizecells, &kinfo->mem); if ( res ) return res; diff --git a/xen/include/public/device_tree_defs.h b/xen/include/public/device_tree_defs.h index 209d43de3f..df58944bd0 100644 --- a/xen/include/public/device_tree_defs.h +++ b/xen/include/public/device_tree_defs.h @@ -7,6 +7,7 @@ * onwards. Reserve a high value for the GIC phandle. */ #define GUEST_PHANDLE_GIC (65000) +#define GUEST_PHANDLE_IOMMU (GUEST_PHANDLE_GIC + 1) #define GUEST_ROOT_ADDRESS_CELLS 2 #define GUEST_ROOT_SIZE_CELLS 2
Currently no IOMMU properties are exposed to dom0, thus kernel by default assumes no protection and enables swiotlb-xen, which leads to costly and unnecessary buffers bouncing. To let kernel know which device is behing IOMMU and hence needs no swiotlb services we introduce dummy xen-iommu node in FDT and link protected device nodes to it, using here device tree iommu bindings. Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> --- Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Julien Grall <julien@xen.org> Cc: Oleksandr Tyshchenko <olekstysh@gmail.com> Cc: Andrii Anisov <Andrii_Anisov@epam.com> Changelog: v3: rebased over staging & remove redundand phandle_iommu attribute, discussion: https://lists.xenproject.org/archives/html/xen-devel/2021-12/msg01753.html v2: re-use common iommu dt bindings to let guests know which devices are protected: https://lists.xenproject.org/archives/html/xen-devel/2021-10/msg00073.html xen/arch/arm/domain_build.c | 42 +++++++++++++++++++++++++++ xen/include/public/device_tree_defs.h | 1 + 2 files changed, 43 insertions(+)