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

[Markus Armbruster]

Daniel Henrique Barboza <danielhb413@gmail.com> writes:

> On 3/28/23 06:53, Markus Armbruster wrote:
>> Daniel Henrique Barboza <danielhb413@gmail.com> writes:

[...]

>>> I believe we can improve the ARM boot code to not create ms->fdt at init(),
>>> leaving it unassigned, and make get_dtb() return the machine FDT on a common
>>> "void *" pointer. That would spare us from having go g_free(ms->fdt) to 
>>> avoid
>>> leaks and we would assign ms->fdt at the end of arm_load_dtb() normally. I 
>>> made
>>> a quick attempt at that but the ARM init() code is a little tricker than 
>>> I've
>>> anticipated. I might have a crack at it later.
>>
>> Do we want a quick interim fix for 8.0?
>> Have a careful look at the untested patch below.
>> 
>>> Thanks,
>>>
>>> Daniel
>>>
>>>
>>>>
>>>>          }
>>>>
>>>>> @@ -689,7 +696,8 @@ int arm_load_dtb(hwaddr addr, const struct 
>>>>> arm_boot_info *binfo,
>>>>>        qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
>>>>>                                           rom_ptr_for_as(as, addr, size));
>>>>>    -    g_free(fdt);
>>>>> +    /* Set ms->fdt for 'dumpdtb' QMP/HMP command */
>>>>> +    ms->fdt = fdt;
>>>>>           return size;
>>>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index 50e5141116..54f6a3e0b3 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -689,7 +689,10 @@ int arm_load_dtb(hwaddr addr, const struct 
>> arm_boot_info *binfo,
>>       qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
>>                                          rom_ptr_for_as(as, addr, size));
>>   -    g_free(fdt);
>> +    if (fdt != ms->fdt) {
>> +        g_free(ms->fdt);
>> +        ms->fdt = fdt;
>> +    }
>>         return size;
>
> This looks better than what I've been proposing here because it centers 
> everything in
> the same spot. It'll also make it easier to change/remove it when we have the 
> chance
> to take a look at the ARM boot code.
>
> Just tested it here and it works fine. Feel free to format it into a patch 
> and send
> it. I'll give my r-b.

I'm going to send it as v3 with your S-o-B, because I'm stealing most of
your commit message.