Re: [PATCH 1/1] hw/arm: do not free machine->fdt in arm_load_dtb()

[Peter Maydell]

On Thu, 23 Mar 2023 at 17:54, Daniel Henrique Barboza
<danielhb413@gmail.com> wrote:
>
>
>
> On 3/23/23 14:38, Peter Maydell wrote:
> > On Thu, 23 Mar 2023 at 16:11, Daniel Henrique Barboza
> > <danielhb413@gmail.com> wrote:
> >> -    g_free(fdt);
> >> +    /* Set ms->fdt for 'dumpdtb' QMP/HMP command */
> >> +    ms->fdt = fdt;
> >
> > With this we're now setting ms->fdt twice for the virt board: we set
> > it in create_fdt() in hw/arm/virt.c, and then we set it again here.
> > Which is the right place to set it?
> >
> > Is the QMP 'dumpdtb' command intended to dump the DTB only for
> > board types where the DTB is created at runtime by QEMU? Or
> > is it supposed to also work for DTBs that were originally
> > provided by the user using the '-dtb' command line? The docs
> > don't say. If we want the former, then we should be setting
> > ms->fdt in the board code; if the latter, then here is right.
>
> My original intent with this command was to dump the current state of the FDT,
> regardless of whether the FDT was loaded via -dtb or at runtime.

Mmm. I think that makes sense; we do make a few tweaks to the DTB
even if it was user-provided and you might want to check those
for debug purposes. So we should keep this assignment, and remove
the now-unneeded setting of ms->fdt in create_fdt().

thanks
-- PMM