Re: [Qemu-devel] [PULL 10/10] hw/arm: versal: Add a virtual Xilinx Versal board

[Edgar E. Iglesias]

Hi Peter,

Thanks for pointing this out. I'll post patches soon unless someone beats
me to it. I'm travelling, hence the slow and top posted reply....

Cheers,
Edgar
---
Sent from my phone

On Tue, Dec 4, 2018, 11:28 Peter Maydell <address@hidden wrote:

> On Fri, 2 Nov 2018 at 17:24, Peter Maydell <address@hidden>
> wrote:
> >
> > From: "Edgar E. Iglesias" <address@hidden>
> >
> > Add a virtual Xilinx Versal board.
> >
> > This board is based on the Xilinx Versal SoC. The exact
> > details of what peripherals are attached to this board
> > will remain in control of QEMU. QEMU will generate an
> > FDT on the fly for Linux and other software to auto-discover
> > peripherals.
> >
> > Signed-off-by: Edgar E. Iglesias <address@hidden>
> > Message-id: address@hidden
> > Reviewed-by: Peter Maydell <address@hidden>
> > Signed-off-by: Peter Maydell <address@hidden>
>
> Hi Edgar -- I've been running the clang-7 leak sanitizer
> on "make check", and it spotted a couple of minor memory
> leaks in the versal board code:
>
> > +static void fdt_add_gic_nodes(VersalVirt *s)
> > +{
> > +    char *nodename;
> > +
> > +    nodename = g_strdup_printf("/address@hidden", MM_GIC_APU_DIST_MAIN);
> > +    qemu_fdt_add_subnode(s->fdt, nodename);
> > +    qemu_fdt_setprop_cell(s->fdt, nodename, "phandle", s->phandle.gic);
> > +    qemu_fdt_setprop_cells(s->fdt, nodename, "interrupts",
> > +                           GIC_FDT_IRQ_TYPE_PPI, VERSAL_GIC_MAINT_IRQ,
> > +                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> > +    qemu_fdt_setprop(s->fdt, nodename, "interrupt-controller", NULL, 0);
> > +    qemu_fdt_setprop_sized_cells(s->fdt, nodename, "reg",
> > +                                 2, MM_GIC_APU_DIST_MAIN,
> > +                                 2, MM_GIC_APU_DIST_MAIN_SIZE,
> > +                                 2, MM_GIC_APU_REDIST_0,
> > +                                 2, MM_GIC_APU_REDIST_0_SIZE);
> > +    qemu_fdt_setprop_cell(s->fdt, nodename, "#interrupt-cells", 3);
> > +    qemu_fdt_setprop_string(s->fdt, nodename, "compatible",
> "arm,gic-v3");
>
> nodename is allocated but never freed here.
>
> > +}
>
>
> > +#define NUM_VIRTIO_TRANSPORT 32
> > +static void create_virtio_regions(VersalVirt *s)
> > +{
> > +    int virtio_mmio_size = 0x200;
> > +    int i;
> > +
> > +    for (i = 0; i < NUM_VIRTIO_TRANSPORT; i++) {
> > +        char *name = g_strdup_printf("virtio%d", i);;
> > +        hwaddr base = MM_TOP_RSVD + i * virtio_mmio_size;
> > +        int irq = VERSAL_RSVD_HIGH_IRQ_FIRST + i;
> > +        MemoryRegion *mr;
> > +        DeviceState *dev;
> > +        qemu_irq pic_irq;
> > +
> > +        pic_irq = qdev_get_gpio_in(DEVICE(&s->soc.fpd.apu.gic), irq);
> > +        dev = qdev_create(NULL, "virtio-mmio");
> > +        object_property_add_child(OBJECT(&s->soc), name, OBJECT(dev),
> > +                                  &error_fatal);
> > +        qdev_init_nofail(dev);
> > +        sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic_irq);
> > +        mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> > +        memory_region_add_subregion(&s->soc.mr_ps, base, mr);
> > +        sysbus_create_simple("virtio-mmio", base, pic_irq);
>
> The body of this loop allocates name but forgets to free it.
>
> > +    }
>
> thanks
> -- PMM
>