diff mbox series

[V4,5/6] libxl: Allocate MMIO params for I2c device and update DT

Message ID 762932ad90785d31039343d543da14c84ce8327d.1660023094.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. 9, 2022, 5:34 a.m. UTC
This patch allocates Virtio MMIO params (IRQ and memory region) and pass
them to the backend, also update Guest device-tree based on Virtio I2C
DT bindings [1].

[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml

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

Comments

Oleksandr Tyshchenko Aug. 18, 2022, 8:50 p.m. UTC | #1
On 09.08.22 08:34, Viresh Kumar wrote:

Hello Viresh

> This patch allocates Virtio MMIO params (IRQ and memory region) and pass
> them to the backend, also update Guest device-tree based on Virtio I2C
> DT bindings [1].

Nit: Patch does more than it claims in the description, I am speaking 
about the changes

related to make_xen_iommu_node(). So I would add a sentence about that here.


>
> [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   tools/libs/light/libxl_arm.c | 57 +++++++++++++++++++++++++++++++-----
>   1 file changed, 50 insertions(+), 7 deletions(-)
>
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 891cb6ef2674..93ea8e3d3fa3 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -110,6 +110,15 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>           }
>       }
>   
> +    for (i = 0; i < d_config->num_i2cs; i++) {
> +        libxl_device_i2c *i2c = &d_config->i2cs[i];
> +

Nit: This blank line is not needed, I think


> +        int rc = alloc_virtio_mmio_params(gc, &i2c->base, &i2c->irq,
> +                &virtio_mmio_base, &virtio_mmio_irq);

Nit: Something wrong with the indentation and looks like the blank line 
is needed here.

> +        if (rc)
> +            return rc;
> +    }
> +
>       /*
>        * Every virtio-mmio device uses one emulated SPI. If Virtio devices are
>        * present, make sure that we allocate enough SPIs for them.
> @@ -947,6 +956,26 @@ static int make_virtio_mmio_node_simple(libxl__gc *gc, void *fdt, uint64_t base,
>       return fdt_end_node(fdt);
>   }
>   
> +static int make_virtio_mmio_node_i2c(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;
> +
> +    res = fdt_begin_node(fdt, "i2c");
> +    if (res) return res;
> +
> +    res = fdt_property_compat(gc, fdt, 1, "virtio,device22");
> +    if (res) return res;
> +
> +    res = fdt_end_node(fdt);
> +    if (res) return res;
> +
> +    return fdt_end_node(fdt);
> +}
> +
>   static const struct arch_info *get_arch_info(libxl__gc *gc,
>                                                const struct xc_dom_image *dom)
>   {
> @@ -1148,7 +1177,7 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
>       size_t fdt_size = 0;
>       int pfdt_size = 0;
>       libxl_domain_build_info *const info = &d_config->b_info;
> -    bool iommu_created;
> +    bool iommu_needed;
>       unsigned int i;
>   
>       const libxl_version_info *vers;
> @@ -1256,22 +1285,36 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
>           if (d_config->num_pcidevs)
>               FDT( make_vpci_node(gc, fdt, ainfo, dom) );
>   
> -        iommu_created = false;
> +        iommu_needed = false;
>           for (i = 0; i < d_config->num_disks; i++) {
>               libxl_device_disk *disk = &d_config->disks[i];
>   
>               if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) {
> -                if (disk->backend_domid != LIBXL_TOOLSTACK_DOMID &&
> -                    !iommu_created) {
> -                    FDT( make_xen_iommu_node(gc, fdt) );
> -                    iommu_created = true;
> -                }
> +                if (disk->backend_domid != LIBXL_TOOLSTACK_DOMID)
> +                    iommu_needed = true;
>   
>                   FDT( make_virtio_mmio_node_simple(gc, fdt, disk->base,
>                                               disk->irq, disk->backend_domid) );
>               }
>           }
>   
> +        for (i = 0; i < d_config->num_i2cs; i++) {
> +            libxl_device_i2c *i2c = &d_config->i2cs[i];
> +
> +            if (i2c->backend_domid != LIBXL_TOOLSTACK_DOMID)
> +                iommu_needed = true;
> +
> +            FDT( make_virtio_mmio_node_i2c(gc, fdt, i2c->base, i2c->irq,
> +                                           i2c->backend_domid) );
> +        }
> +
> +        /*
> +         * Note, this should be only called after creating all virtio-mmio
> +         * device nodes
> +         */
> +        if (iommu_needed)
> +            FDT( make_xen_iommu_node(gc, fdt) );
> +
>           if (pfdt)
>               FDT( copy_partial_fdt(gc, fdt, pfdt) );


Preferably with above fixed:


Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>


>
diff mbox series

Patch

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 891cb6ef2674..93ea8e3d3fa3 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -110,6 +110,15 @@  int libxl__arch_domain_prepare_config(libxl__gc *gc,
         }
     }
 
+    for (i = 0; i < d_config->num_i2cs; i++) {
+        libxl_device_i2c *i2c = &d_config->i2cs[i];
+
+        int rc = alloc_virtio_mmio_params(gc, &i2c->base, &i2c->irq,
+                &virtio_mmio_base, &virtio_mmio_irq);
+        if (rc)
+            return rc;
+    }
+
     /*
      * Every virtio-mmio device uses one emulated SPI. If Virtio devices are
      * present, make sure that we allocate enough SPIs for them.
@@ -947,6 +956,26 @@  static int make_virtio_mmio_node_simple(libxl__gc *gc, void *fdt, uint64_t base,
     return fdt_end_node(fdt);
 }
 
+static int make_virtio_mmio_node_i2c(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;
+
+    res = fdt_begin_node(fdt, "i2c");
+    if (res) return res;
+
+    res = fdt_property_compat(gc, fdt, 1, "virtio,device22");
+    if (res) return res;
+
+    res = fdt_end_node(fdt);
+    if (res) return res;
+
+    return fdt_end_node(fdt);
+}
+
 static const struct arch_info *get_arch_info(libxl__gc *gc,
                                              const struct xc_dom_image *dom)
 {
@@ -1148,7 +1177,7 @@  static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
     size_t fdt_size = 0;
     int pfdt_size = 0;
     libxl_domain_build_info *const info = &d_config->b_info;
-    bool iommu_created;
+    bool iommu_needed;
     unsigned int i;
 
     const libxl_version_info *vers;
@@ -1256,22 +1285,36 @@  static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
         if (d_config->num_pcidevs)
             FDT( make_vpci_node(gc, fdt, ainfo, dom) );
 
-        iommu_created = false;
+        iommu_needed = false;
         for (i = 0; i < d_config->num_disks; i++) {
             libxl_device_disk *disk = &d_config->disks[i];
 
             if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) {
-                if (disk->backend_domid != LIBXL_TOOLSTACK_DOMID &&
-                    !iommu_created) {
-                    FDT( make_xen_iommu_node(gc, fdt) );
-                    iommu_created = true;
-                }
+                if (disk->backend_domid != LIBXL_TOOLSTACK_DOMID)
+                    iommu_needed = true;
 
                 FDT( make_virtio_mmio_node_simple(gc, fdt, disk->base,
                                             disk->irq, disk->backend_domid) );
             }
         }
 
+        for (i = 0; i < d_config->num_i2cs; i++) {
+            libxl_device_i2c *i2c = &d_config->i2cs[i];
+
+            if (i2c->backend_domid != LIBXL_TOOLSTACK_DOMID)
+                iommu_needed = true;
+
+            FDT( make_virtio_mmio_node_i2c(gc, fdt, i2c->base, i2c->irq,
+                                           i2c->backend_domid) );
+        }
+
+        /*
+         * Note, this should be only called after creating all virtio-mmio
+         * device nodes
+         */
+        if (iommu_needed)
+            FDT( make_xen_iommu_node(gc, fdt) );
+
         if (pfdt)
             FDT( copy_partial_fdt(gc, fdt, pfdt) );