On 26/02/2025 11:59, Luca Fancellu wrote:
>
>
>>>
>>>>
>>>>>
>>>>> return fdt;
>>>>> }
>>>>> @@ -362,7 +363,7 @@ void asmlinkage __init start_xen(unsigned long
>>>>> fdt_paddr)
>>>>> if ( acpi_disabled )
>>>>> {
>>>>> printk("Booting using Device Tree\n");
>>>>> - device_tree_flattened = relocate_fdt(fdt_paddr, fdt_size);
>>>>> + device_tree_flattened = relocate_fdt(device_tree_flattened,
>>>>> fdt_size);
>>>> NIT: It can be just my PoV but it reads odd. Why can't relocate_fdt modify
>>>> device_tree_flattened pointer directly in the function?
>>>
>>> you mean something like:
>>>
>>> static void * __init relocate_fdt(size_t dtb_size)
>>> {
>>> void *fdt = xmalloc_bytes(dtb_size);
>>>
>>> if ( !fdt )
>>> panic("Unable to allocate memory for relocating the Device-Tree.\n");
>>>
>>> memcpy(fdt, device_tree_flattened, dtb_size);
>> You already make assumption about device_tree_flattened being global, so why
>> not assigning device_tree_flattened = fdt in the function as well?
>
> just because it’s more easy to follow the global variable changes when
> reading the start_xen(…)
> code as the function is the only one modifying it.
>
> If you strongly oppose to that I’ll change, but imo it’s easier to follow the
> code in this way
How about:
static void __init relocate_fdt(void *dtb_vaddr, size_t dtb_size)
{
void *fdt = xmalloc_bytes(dtb_size);
if ( !fdt )
panic("Unable to allocate memory for relocating the Device-Tree.\n");
memcpy(fdt, dtb_vaddr, dtb_size);
clean_dcache_va_range(fdt, dtb_size);
dtb_vaddr = fdt;
}
This would be best IMO. That said, I don't care that much. Choose whatever makes
sense.
~Michal
