On Tue, 2 Nov 2021 at 16:48, Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote:
>
> Allows edk2 detect virtio-mmio devices and pcie ecam.
> See comment in hw/i386/microvm-dt.c for more details.
>
> Signed-off-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> Reviewed-by: Sergio Lopez <slp@xxxxxxxxxx>
> Message-Id: <20211014193617.2475578-1-kraxel@xxxxxxxxxx>
Hi; Coverity points out a couple of issues in this code.
> +static void dt_add_pcie(MicrovmMachineState *mms)
> +{
> + hwaddr base = PCIE_MMIO_BASE;
> + int nr_pcie_buses;
> + char *nodename;
> +
> + nodename = g_strdup_printf("/pcie@%" PRIx64, base);
Here we allocate 'nodename'...
> + qemu_fdt_add_subnode(mms->fdt, nodename);
> + qemu_fdt_setprop_string(mms->fdt, nodename,
> + "compatible", "pci-host-ecam-generic");
> + qemu_fdt_setprop_string(mms->fdt, nodename, "device_type", "pci");
> + qemu_fdt_setprop_cell(mms->fdt, nodename, "#address-cells", 3);
> + qemu_fdt_setprop_cell(mms->fdt, nodename, "#size-cells", 2);
> + qemu_fdt_setprop_cell(mms->fdt, nodename, "linux,pci-domain", 0);
> + qemu_fdt_setprop(mms->fdt, nodename, "dma-coherent", NULL, 0);
> +
> + qemu_fdt_setprop_sized_cells(mms->fdt, nodename, "reg",
> + 2, PCIE_ECAM_BASE, 2, PCIE_ECAM_SIZE);
> + if (mms->gpex.mmio64.size) {
> + qemu_fdt_setprop_sized_cells(mms->fdt, nodename, "ranges",
> +
> + 1, FDT_PCI_RANGE_MMIO,
> + 2, mms->gpex.mmio32.base,
> + 2, mms->gpex.mmio32.base,
> + 2, mms->gpex.mmio32.size,
> +
> + 1, FDT_PCI_RANGE_MMIO_64BIT,
> + 2, mms->gpex.mmio64.base,
> + 2, mms->gpex.mmio64.base,
> + 2, mms->gpex.mmio64.size);
> + } else {
> + qemu_fdt_setprop_sized_cells(mms->fdt, nodename, "ranges",
> +
> + 1, FDT_PCI_RANGE_MMIO,
> + 2, mms->gpex.mmio32.base,
> + 2, mms->gpex.mmio32.base,
> + 2, mms->gpex.mmio32.size);
> + }
> +
> + nr_pcie_buses = PCIE_ECAM_SIZE / PCIE_MMCFG_SIZE_MIN;
> + qemu_fdt_setprop_cells(mms->fdt, nodename, "bus-range", 0,
> + nr_pcie_buses - 1);
...but unlike other functions in this file we forget to free it.
(CID 1465240)
Might be worth considering use of g_autofree.
> +}
> +void dt_setup_microvm(MicrovmMachineState *mms)
> +{
> + X86MachineState *x86ms = X86_MACHINE(mms);
> + int size = 0;
> +
> + mms->fdt = create_device_tree(&size);
> +
> + /* root node */
> + qemu_fdt_setprop_string(mms->fdt, "/", "compatible", "linux,microvm");
> + qemu_fdt_setprop_cell(mms->fdt, "/", "#address-cells", 0x2);
> + qemu_fdt_setprop_cell(mms->fdt, "/", "#size-cells", 0x2);
> +
> + qemu_fdt_add_subnode(mms->fdt, "/chosen");
> + dt_setup_sys_bus(mms);
> +
> + /* add to fw_cfg */
> + fprintf(stderr, "%s: add etc/fdt to fw_cfg\n", __func__);
> + fw_cfg_add_file(x86ms->fw_cfg, "etc/fdt", mms->fdt, size);
> +
> + if (debug) {
> + fprintf(stderr, "%s: writing microvm.fdt\n", __func__);
> + g_file_set_contents("microvm.fdt", mms->fdt, size, NULL);
g_file_set_contents() returns a value to indicate success or
failure; we should check it.
(CID 1465239)
> + int ret = system("dtc -I dtb -O dts microvm.fdt");
> + if (ret != 0) {
> + fprintf(stderr, "%s: oops, dtc not installed?\n", __func__);
> + }
> + }
> +}
thanks
-- PMM
