diff mbox series

[V3,4/6] libxl: arm: Split make_virtio_mmio_node()

Message ID ae20b87d583b981302905059d157a03400cd8eb2.1659596139.git.viresh.kumar@linaro.org (mailing list archive)
State Superseded
Headers show
Series Virtio toolstack support for I2C and GPIO on Arm | expand

Commit Message

Viresh Kumar Aug. 4, 2022, 7:01 a.m. UTC
make_virtio_mmio_node() creates the DT node for simple MMIO devices
currently, i.e. the ones that don't require any additional properties.

In order to allow using it for other complex device types, split the
functionality into two, one where the fdt node isn't closed and the
other one to create a simple DT node.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 tools/libs/light/libxl_arm.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

Comments

Oleksandr Tyshchenko Aug. 8, 2022, 7:32 p.m. UTC | #1
On 04.08.22 10:01, Viresh Kumar wrote:

Hello Viresh

> make_virtio_mmio_node() creates the DT node for simple MMIO devices
> currently, i.e. the ones that don't require any additional properties.
>
> In order to allow using it for other complex device types, split the
> functionality into two, one where the fdt node isn't closed and the
> other one to create a simple DT node.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   tools/libs/light/libxl_arm.c | 24 ++++++++++++++----------
>   1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 2f64b9f0ebee..6a8c4d042bd9 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -900,9 +900,8 @@ static int make_xen_iommu_node(libxl__gc *gc, void *fdt)
>       return 0;
>   }
>   
> -static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
> -                                 uint64_t base, uint32_t irq,
> -                                 uint32_t backend_domid)
> +static int make_virtio_mmio_node_common(libxl__gc *gc, void *fdt, uint64_t base,
> +                                        uint32_t irq)
>   {
>       int res;
>       gic_interrupt intr;
> @@ -922,7 +921,15 @@ static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
>       res = fdt_property_interrupts(gc, fdt, &intr, 1);
>       if (res) return res;
>   
> -    res = fdt_property(fdt, "dma-coherent", NULL, 0);
> +    return fdt_property(fdt, "dma-coherent", NULL, 0);
> +}
> +
> +static int make_virtio_mmio_node_simple(libxl__gc *gc, void *fdt, uint64_t base,
> +                                        uint32_t irq, uint32_t backend_domid)
> +{
> +    int res;
> +
> +    res = make_virtio_mmio_node_common(gc, fdt, base, irq);
>       if (res) return res;
>   
>       if (backend_domid != LIBXL_TOOLSTACK_DOMID) {
> @@ -935,10 +942,7 @@ static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
>           if (res) return res;
>       }
>   
> -    res = fdt_end_node(fdt);
> -    if (res) return res;
> -
> -    return 0;
> +    return fdt_end_node(fdt);
>   }
>   
>   static const struct arch_info *get_arch_info(libxl__gc *gc,
> @@ -1261,8 +1265,8 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
>                       iommu_created = true;
>                   }
>   
> -                FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq,
> -                                           disk->backend_domid) );
> +                FDT( make_virtio_mmio_node_simple(gc, fdt, disk->base,
> +                                            disk->irq, disk->backend_domid) );
>               }
>           }

[I failed to find a better place where to make a comment]


I think that the following chunk should be also moved to 
make_virtio_mmio_node_common() since it is not a virtio disk specific 
action, this "iommus" property pointing to "xen,grant-dma" IOMMU node is 
used to inform a guest that restricted memory access using Xen grant 
mappings needs to be enabled for the virtio device if it's backend is 
going to run in a non hardware domain, which I assume also applies for 
virtio i2c, etc.


     if (backend_domid != LIBXL_TOOLSTACK_DOMID) {
         uint32_t iommus_prop[2];

         iommus_prop[0] = cpu_to_fdt32(GUEST_PHANDLE_IOMMU);
         iommus_prop[1] = cpu_to_fdt32(backend_domid);

         res = fdt_property(fdt, "iommus", iommus_prop, 
sizeof(iommus_prop));
         if (res) return res;
     }


This means that "uint32_t backend_domid" should be also passed as 
argument to make_virtio_mmio_node_common()


Something like the diff on top of current patch below (not tested):

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index a98cfe708b..1a6ace3d8d 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -900,7 +900,7 @@ static int make_xen_iommu_node(libxl__gc *gc, void *fdt)
  }

  static int make_virtio_mmio_node_common(libxl__gc *gc, void *fdt, 
uint64_t base,
-                                        uint32_t irq)
+                                        uint32_t irq, uint32_t 
backend_domid)
  {
      int res;
      gic_interrupt intr;
@@ -920,15 +920,7 @@ static int make_virtio_mmio_node_common(libxl__gc 
*gc, void *fdt, uint64_t base,
      res = fdt_property_interrupts(gc, fdt, &intr, 1);
      if (res) return res;

-    return fdt_property(fdt, "dma-coherent", NULL, 0);
-}
-
-static int make_virtio_mmio_node_simple(libxl__gc *gc, void *fdt, 
uint64_t base,
-                                        uint32_t irq, uint32_t 
backend_domid)
-{
-    int res;
-
-    res = make_virtio_mmio_node_common(gc, fdt, base, irq);
+    res = fdt_property(fdt, "dma-coherent", NULL, 0);
      if (res) return res;

      if (backend_domid != LIBXL_TOOLSTACK_DOMID) {
@@ -941,6 +933,17 @@ static int make_virtio_mmio_node_simple(libxl__gc 
*gc, void *fdt, uint64_t base,
          if (res) return res;
      }

+    return res;
+}
+
+static int make_virtio_mmio_node_simple(libxl__gc *gc, void *fdt, 
uint64_t base,
+                                        uint32_t irq, uint32_t 
backend_domid)
+{
+    int res;
+
+    res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid);
+    if (res) return res;
+
      return fdt_end_node(fdt);
  }


Other changes look good.


>
diff mbox series

Patch

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 2f64b9f0ebee..6a8c4d042bd9 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -900,9 +900,8 @@  static int make_xen_iommu_node(libxl__gc *gc, void *fdt)
     return 0;
 }
 
-static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
-                                 uint64_t base, uint32_t irq,
-                                 uint32_t backend_domid)
+static int make_virtio_mmio_node_common(libxl__gc *gc, void *fdt, uint64_t base,
+                                        uint32_t irq)
 {
     int res;
     gic_interrupt intr;
@@ -922,7 +921,15 @@  static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
     res = fdt_property_interrupts(gc, fdt, &intr, 1);
     if (res) return res;
 
-    res = fdt_property(fdt, "dma-coherent", NULL, 0);
+    return fdt_property(fdt, "dma-coherent", NULL, 0);
+}
+
+static int make_virtio_mmio_node_simple(libxl__gc *gc, void *fdt, uint64_t base,
+                                        uint32_t irq, uint32_t backend_domid)
+{
+    int res;
+
+    res = make_virtio_mmio_node_common(gc, fdt, base, irq);
     if (res) return res;
 
     if (backend_domid != LIBXL_TOOLSTACK_DOMID) {
@@ -935,10 +942,7 @@  static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
         if (res) return res;
     }
 
-    res = fdt_end_node(fdt);
-    if (res) return res;
-
-    return 0;
+    return fdt_end_node(fdt);
 }
 
 static const struct arch_info *get_arch_info(libxl__gc *gc,
@@ -1261,8 +1265,8 @@  static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
                     iommu_created = true;
                 }
 
-                FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq,
-                                           disk->backend_domid) );
+                FDT( make_virtio_mmio_node_simple(gc, fdt, disk->base,
+                                            disk->irq, disk->backend_domid) );
             }
         }