On Tue, Jul 13, 2021 at 8:44 PM Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Tue, 14 Jul 2020 at 01:44, Alistair Francis <alistair.fran...@wdc.com>
> wrote:
> >
> > From: Atish Patra <atish.pa...@wdc.com>
> >
> > Currently, the fdt is copied to the ROM after the reset vector. The firmware
> > has to copy it to DRAM. Instead of this, directly copy the device tree to a
> > pre-computed dram address. The device tree load address should be as far as
> > possible from kernel and initrd images. That's why it is kept at the end of
> > the DRAM or 4GB whichever is lesser.
>
> Hi; Coverity reports an issue in this code (CID 1458136):
>
> > +uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
> > +{
> > + uint32_t temp, fdt_addr;
> > + hwaddr dram_end = dram_base + mem_size;
> > + int fdtsize = fdt_totalsize(fdt);
> > +
> > + if (fdtsize <= 0) {
> > + error_report("invalid device-tree");
> > + exit(1);
> > + }
> > +
> > + /*
> > + * We should put fdt as far as possible to avoid kernel/initrd
> > overwriting
> > + * its content. But it should be addressable by 32 bit system as well.
> > + * Thus, put it at an aligned address that less than fdt size from end
> > of
> > + * dram or 4GB whichever is lesser.
> > + */
> > + temp = MIN(dram_end, 4096 * MiB);
> > + fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> > +
> > + fdt_pack(fdt);
>
> fdt_pack() can return an error code, but we are not checking its
> return value here.
>
> (This is one of Coverity's heuristics where it only reports failure
> to check errors if it sees enough other callsites in the codebase
> which do check errors to make it decide this is an "always need a
> check" API, which is why the error has only popped up now a year on...)
Thanks Peter, sending a patch now.
Alistair
>
> > + /* copy in the device tree */
> > + qemu_fdt_dumpdtb(fdt, fdtsize);
> > +
> > + rom_add_blob_fixed_as("fdt", fdt, fdtsize, fdt_addr,
> > + &address_space_memory);
> > +
> > + return fdt_addr;
> > +}
>
> thanks
> -- PMM
>
