Re: [XEN v5 02/10] xen/arm: Typecast the DT values into paddr_t

[Michal Orzel <michal.orzel@xxxxxxx>]

On 19/04/2023 15:19, Michal Orzel wrote:
> 
> 
> Hi Ayan,
> 
> On 13/04/2023 19:37, Ayan Kumar Halder wrote:
>>
>>
>> The DT functions (dt_read_number(), device_tree_get_reg(), fdt_get_mem_rsv())
>> currently accept or return 64-bit values.
>>
>> In future when we support 32-bit physical address, these DT functions are
>> expected to accept/return 32-bit or 64-bit values (depending on the width of
>> physical address). Also, we wish to detect if any truncation has occurred
>> (i.e. while parsing 32-bit physical addresses from 64-bit values read from 
>> DT).
>>
>> device_tree_get_reg() should now be able to return paddr_t. This is invoked 
>> by
>> various callers to get DT address and size.
>>
>> For fdt_get_mem_rsv(), we have introduced a wrapper named
>> fdt_get_mem_rsv_paddr() which will invoke fdt_get_mem_rsv() and translate
>> uint64_t to paddr_t. The reason being we cannot modify fdt_get_mem_rsv() as 
>> it
>> has been imported from external source.
>>
>> For dt_read_number(), we have also introduced a wrapper named dt_read_paddr()
>> dt_read_paddr() to read physical addresses. We chose not to modify the 
>> original
>> function as it is used in places where it needs to specifically read 64-bit
>> values from dt (For e.g. dt_property_read_u64()).
>>
>> Xen prints warning when it detects truncation in cases where it is not able 
>> to
>> return error.
>>
>> Also, replaced u32/u64 with uint32_t/uint64_t in the functions touched
>> by the code changes.
>>
>> Also, initialized variables to fix the warning "-Werror=maybe-uninitialized".
> I can see that now you explicitly set to 0 variables passed to 
> fdt_get_mem_rsv_paddr()
> which haven't been initialized before being passed to fdt_get_mem_rsv(). Is 
> this what
> you are reffering to? I cannot reproduce it, hence my question.
I can see why did you get this error.
Before your change, we always checked for an error from fdt_get_mem_rsv() by 
checking if < 0.
In your wrapper fdt_get_mem_rsv_paddr(), you switched (not sure why) to 
checking if not zero.
Becasue of this, you got an error and tried to fix it by initializing the 
variables to 0.

~Michal