Re: QMP command dumpdtb crash bug

[Bernhard Beschow]

Am 23. März 2023 15:13:28 UTC schrieb Daniel Henrique Barboza 
<dbarboza@ventanamicro.com>:
>
>
>On 3/23/23 10:38, Peter Maydell wrote:
>> On Thu, 23 Mar 2023 at 13:29, Markus Armbruster <armbru@redhat.com> wrote:
>>> 
>>> Peter, Daniel offers two ways to fix this bug (see below).  Got a
>>> preference?
>> 
>> Not freeing seems the correct thing. As Daniel says, this
>> should have been a prerequisite for implementing the
>> command in the first place (you need to change the lifecycle
>> of the fdt blob from "delete when done with in the arm boot code"
>> to "delete on machine finalize"). It looks like somehow we added
>> the command but missed out on getting all of the prerequisite
>> patches in. (File under "need to be cautious about applying partial
>> patchsets", I guess.)
>
>Yeah, I'm at fault here. I should've been more insistent about acking
>the ARM patch. All other patches that we left behind was optional, meaning
>that the machine wouldn't implement the command but nothing bad would happen,
>but the ARM patch was kind of mandatory because arm_load_dtb() is
>freeing ms->fdt without assigning it to NULL.
>
>> 
>> Did anything else from that initial patchset get omitted?
>
>Searching the ML I see that I sent a message saying that I pushed patches 1,
>6 and 8-15 via ppc-next. This means that these patches got left behind:
>
> 2  hw/core: free ms->fdt in machine_finalize()
> 3  hw/arm: do not free machine->fdt in arm_load_dtb()
> 4  hw/mips: set machine->fdt in boston_mach_init()
> 5  hw/microblaze: set machine->fdt in microblaze_load_dtb()
> 7  hw/ppc: set machine->fdt in ppce500_load_device_tree()

We dealt with e500 in a different series. So 7 is basically in 8.0 already (but 
in a different form).

Best regards,
Bernhard

>15  hw/xtensa: set machine->fdt in xtfpga_init()
>
>
>Patch 2 was suggested by Phil and changes the common code to free ms->fdt
>during machine_finalize(). Can be re-sent I guess.
>
>All other patches, aside from patch 3 from ARM, are optional because the
>machine isn't freeing ms->fdt or anything like that.
>
>
>I'll rebase and re-sent patch 3 as a bug fix. I'll re-sent the hw/core patch
>as well for 8.1.
>
>
>Daniel
>
>
>> 
>> thanks
>> -- PMM
>