[Stratos-dev] Re: [PATCH 3/4] libxl: Allocate MMIO params for I2c device and update DT

On 05.05.22 10:33, 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.

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

tools/libs/light/libxl_arm.c | 57 ++++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 8132a47a72b6..ea633d6f91df 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -115,6 +115,26 @@ 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];
  

•    i2c->base = alloc_virtio_mmio_base(gc);
  

•    if (!i2c->base)
  

•        return ERROR_FAIL;
  

•    i2c->irq = alloc_virtio_mmio_irq(gc);
  

•    if (!i2c->irq)
  

•        return ERROR_FAIL;
  

•    if (virtio_irq < i2c->irq)
  

•        virtio_irq = i2c->irq;
  

•    virtio_enabled = true;
  

•    LOG(DEBUG, "Allocate Virtio MMIO params for I2C: IRQ %u BASE 0x%"PRIx64,
  

•        i2c->irq, i2c->base);
  

• }

• if (virtio_enabled)
      nr_spis += (virtio_irq - 32) + 1;
  

@@ -874,7 +894,7 @@ static int make_vpci_node(libxl__gc *gc, void *fdt, } -static int make_virtio_mmio_node(libxl__gc *gc, void *fdt, +static int _make_virtio_mmio_node(libxl__gc *gc, void *fdt, uint64_t base, uint32_t irq)

NIT: I am not sure the leading underscore in the name would be welcome. I would probably rename it.

How about s/_make_virtio_mmio_node/make_virtio_mmio_node_common here and ...

{ int res; @@ -897,7 +917,35 @@ 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(libxl__gc *gc, void *fdt,

•                             uint64_t base, uint32_t irq)
  

... s/make_virtio_mmio_node/make_virtio_mmio_node_simple here or something like that?

+{

• int res;
• res = _make_virtio_mmio_node(gc, fdt, base, irq);
• if (res) return res;
• 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(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");

Please provide a link (in the commit description) to the corresponding Linux device tree binding. I assume it is:

Documentation/devicetree/bindings/i2c/i2c-virtio.yaml

• if (res) return res;
• res = fdt_end_node(fdt); if (res) return res;

res = fdt_end_node(fdt); @@ -1221,6 +1269,11 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config, FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq) ); }

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