| Message ID | 0b8d38ef26bfa9bc150f3818108ca9e875652e5e.1651734854.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Virtio toolstack support for I2C and GPIO on Arm | expand |
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 | 60 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 60 insertions(+) > > diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c > index ea633d6f91df..89e5a1e5780d 100644 > --- a/tools/libs/light/libxl_arm.c > +++ b/tools/libs/light/libxl_arm.c > @@ -135,6 +135,26 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, > i2c->irq, i2c->base); > } > > + for (i = 0; i < d_config->num_gpios; i++) { > + libxl_device_gpio *gpio = &d_config->gpios[i]; > + > + gpio->base = alloc_virtio_mmio_base(gc); > + if (!gpio->base) > + return ERROR_FAIL; > + > + gpio->irq = alloc_virtio_mmio_irq(gc); > + if (!gpio->irq) > + return ERROR_FAIL; > + > + if (virtio_irq < gpio->irq) > + virtio_irq = gpio->irq; > + > + virtio_enabled = true; > + > + LOG(DEBUG, "Allocate Virtio MMIO params for GPIO: IRQ %u BASE 0x%"PRIx64, > + gpio->irq, gpio->base); > + } > + Looks like, we are going to end up with some duplication in libxl__arch_domain_prepare_config(). This is already a third chunk (the second was in patch #3/4). I would probably consider moving some code to a separate function. Below the non-tested diff (based on the recent V8 patch [1]), how it could look like. I am not sure this is an ideal variant, but rather to demonstrate what I mean. diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 25969e0..6e23c26 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -48,6 +48,23 @@ static uint32_t alloc_virtio_mmio_irq(libxl__gc *gc, uint32_t *virtio_mmio_irq) return irq; } +static int alloc_virtio_mmio_params(libxl__gc *gc, uint64_t *base, uint32_t *irq, + uint64_t *virtio_mmio_base, + uint32_t *virtio_mmio_irq) +{ + *base = alloc_virtio_mmio_base(gc, virtio_mmio_base); + if (!*base) + return ERROR_FAIL; + + *irq = alloc_virtio_mmio_irq(gc, virtio_mmio_irq); + if (!*irq) + return ERROR_FAIL; + + LOG(DEBUG, "Allocate Virtio MMIO params: IRQ %u BASE 0x%"PRIx64, *irq, *base); + + return 0; +} + static const char *gicv_to_string(libxl_gic_version gic_version) { switch (gic_version) { @@ -85,25 +102,18 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, libxl_device_disk *disk = &d_config->disks[i]; if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) { - disk->base = alloc_virtio_mmio_base(gc, &virtio_mmio_base); - if (!disk->base) - return ERROR_FAIL; - - disk->irq = alloc_virtio_mmio_irq(gc, &virtio_mmio_irq); - if (!disk->irq) - return ERROR_FAIL; - - if (virtio_irq < disk->irq) - virtio_irq = disk->irq; - virtio_enabled = true; - - LOG(DEBUG, "Allocate Virtio MMIO params for Vdev %s: IRQ %u BASE 0x%"PRIx64, - disk->vdev, disk->irq, disk->base); + int rc = alloc_virtio_mmio_params(gc, &disk->base, &disk->irq, + &virtio_mmio_base, &virtio_mmio_irq); + if (rc) + return rc; } } - if (virtio_enabled) + if (virtio_mmio_irq != GUEST_VIRTIO_MMIO_SPI_FIRST) { + virtio_irq = virtio_mmio_irq - 1; nr_spis += (virtio_irq - 32) + 1; + virtio_enabled = true; + } for (i = 0; i < d_config->b_info.num_irqs; i++) { uint32_t irq = d_config->b_info.irqs[i]; (END) So, if this was done before (in a prereq patch), your new addition here would be just the following: diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 6e23c26..ae64cbd 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -109,6 +109,14 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, } } + for (i = 0; i < d_config->num_gpios; i++) { + libxl_device_gpio *gpio = &d_config->gpios[i]; + int rc = alloc_virtio_mmio_params(gc, &gpio->base, &gpio->irq, + &virtio_mmio_base, &virtio_mmio_irq); + if (rc) + return rc; + } + if (virtio_mmio_irq != GUEST_VIRTIO_MMIO_SPI_FIRST) { virtio_irq = virtio_mmio_irq - 1; nr_spis += (virtio_irq - 32) + 1; > if (virtio_enabled) > nr_spis += (virtio_irq - 32) + 1; > > @@ -954,6 +974,41 @@ static int make_virtio_mmio_node_i2c(libxl__gc *gc, void *fdt, > return 0; > } > > +static int make_virtio_mmio_node_gpio(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, "gpio"); > + if (res) return res; > + > + res = fdt_property_compat(gc, fdt, 1, "virtio,device29"); Please provide a link (in the commit description) to the corresponding Linux device tree binding. I assume it is: Documentation/devicetree/bindings/gpio/gpio-virtio.yaml > + if (res) return res; > + > + res = fdt_property(fdt, "gpio-controller", NULL, 0); > + if (res) return res; > + > + res = fdt_property_cell(fdt, "#gpio-cells", 2); > + if (res) return res; > + > + res = fdt_property(fdt, "interrupt-controller", NULL, 0); > + if (res) return res; > + > + res = fdt_property_cell(fdt, "#interrupt-cells", 2); > + if (res) return res; > + > + res = fdt_end_node(fdt); > + if (res) return res; > + > + res = fdt_end_node(fdt); > + if (res) return res; > + > + return 0; > +} > + > static const struct arch_info *get_arch_info(libxl__gc *gc, > const struct xc_dom_image *dom) > { > @@ -1269,6 +1324,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_gpios; i++) { > + libxl_device_gpio *gpio = &d_config->gpios[i]; > + FDT( make_virtio_mmio_node_gpio(gc, fdt, gpio->base, gpio->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) ); [1] https://lore.kernel.org/xen-devel/1651598763-12162-3-git-send-email-olekstysh@gmail.com/
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index ea633d6f91df..89e5a1e5780d 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -135,6 +135,26 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, i2c->irq, i2c->base); } + for (i = 0; i < d_config->num_gpios; i++) { + libxl_device_gpio *gpio = &d_config->gpios[i]; + + gpio->base = alloc_virtio_mmio_base(gc); + if (!gpio->base) + return ERROR_FAIL; + + gpio->irq = alloc_virtio_mmio_irq(gc); + if (!gpio->irq) + return ERROR_FAIL; + + if (virtio_irq < gpio->irq) + virtio_irq = gpio->irq; + + virtio_enabled = true; + + LOG(DEBUG, "Allocate Virtio MMIO params for GPIO: IRQ %u BASE 0x%"PRIx64, + gpio->irq, gpio->base); + } + if (virtio_enabled) nr_spis += (virtio_irq - 32) + 1; @@ -954,6 +974,41 @@ static int make_virtio_mmio_node_i2c(libxl__gc *gc, void *fdt, return 0; } +static int make_virtio_mmio_node_gpio(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, "gpio"); + if (res) return res; + + res = fdt_property_compat(gc, fdt, 1, "virtio,device29"); + if (res) return res; + + res = fdt_property(fdt, "gpio-controller", NULL, 0); + if (res) return res; + + res = fdt_property_cell(fdt, "#gpio-cells", 2); + if (res) return res; + + res = fdt_property(fdt, "interrupt-controller", NULL, 0); + if (res) return res; + + res = fdt_property_cell(fdt, "#interrupt-cells", 2); + if (res) return res; + + res = fdt_end_node(fdt); + if (res) return res; + + res = fdt_end_node(fdt); + if (res) return res; + + return 0; +} + static const struct arch_info *get_arch_info(libxl__gc *gc, const struct xc_dom_image *dom) { @@ -1269,6 +1324,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_gpios; i++) { + libxl_device_gpio *gpio = &d_config->gpios[i]; + FDT( make_virtio_mmio_node_gpio(gc, fdt, gpio->base, gpio->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) );
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 | 60 ++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)