[Stratos-dev] Re: [PATCH V3 5/6] libxl: Allocate MMIO params for I2c device and update DT

On 04.08.22 10:01, 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].

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

Signed-off-by: Viresh Kumar viresh.kumar@linaro.org

tools/libs/light/libxl_arm.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 6a8c4d042bd9..08a1499c9523 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -112,6 +112,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.
  

@@ -945,6 +954,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)
  

+{

• int res;
• res = make_virtio_mmio_node_common(gc, fdt, base, irq);
• 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) {

@@ -1270,6 +1299,12 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config, } }

•    for (i = 0; i < d_config->num_i2cs; i++) {
  

•        libxl_device_i2c *i2c = &d_config->i2cs[i];
  

•        FDT( make_virtio_mmio_node_i2c(gc, fdt, i2c->base, i2c->irq) );
  

•    }
  

•     if (pfdt)
          FDT( copy_partial_fdt(gc, fdt, pfdt) );
  

I think that patch needs to be updated according to the suggestion provided for "[PATCH V3 4/6] libxl: arm: Split make_virtio_mmio_node()" (of course, if you agree with it).

If so, the make_virtio_mmio_node_i2c() should gain "uint32_t backend_domid" argument, etc. And with introducing new virtio i2c the make_xen_iommu_node() should also be called for it if not created yet (or maybe better to have only single invocation of make_xen_iommu_node() at the end)

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 0e448c486b..725ccb5f3f 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -957,11 +957,11 @@ static int make_virtio_mmio_node_simple(libxl__gc *gc, void *fdt, uint64_t base,  }

static int make_virtio_mmio_node_i2c(libxl__gc *gc, void *fdt, uint64_t base, -                                     uint32_t irq) +                                     uint32_t irq, uint32_t backend_domid)  {      int res;

-    res = make_virtio_mmio_node_common(gc, fdt, base, irq); +    res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid);      if (res) return res;

res = fdt_begin_node(fdt, "i2c"); @@ -1177,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; @@ -1285,16 +1285,13 @@ next_resize:          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) ); @@ -1303,10 +1300,20 @@ next_resize:

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) ); +            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) );

(END)

Other changes look good.