[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/1] hw/arm: do not free machine->fdt in arm_load_dtb()
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 1/1] hw/arm: do not free machine->fdt in arm_load_dtb() |
Date: |
Tue, 28 Mar 2023 18:58:42 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
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.