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 <[email protected] wrote: > On Fri, 2 Nov 2018 at 17:24, Peter Maydell <[email protected]> > wrote: > > > > From: "Edgar E. Iglesias" <[email protected]> > > > > 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 <[email protected]> > > Message-id: [email protected] > > Reviewed-by: Peter Maydell <[email protected]> > > Signed-off-by: Peter Maydell <[email protected]> > > 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("/gic@%x", 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 >
