[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN v5 02/10] xen/arm: Typecast the DT values into paddr_t
On 19/04/2023 16:18, Julien Grall wrote: Hi Ayan, Hi Julien, On 19/04/2023 15:58, Ayan Kumar Halder wrote:On 19/04/2023 14:54, Michal Orzel wrote:On 19/04/2023 15:19, Michal Orzel wrote:Hi Ayan, On 13/04/2023 19:37, Ayan Kumar Halder wrote: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 whatThe 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 byvarious callers to get DT address and size. For fdt_get_mem_rsv(), we have introduced a wrapper namedfdt_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 ithas 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-bitvalues from dt (For e.g. dt_property_read_u64()).Xen prints warning when it detects truncation in cases where it is not able toreturn error.Also, replaced u32/u64 with uint32_t/uint64_t in the functions touchedby the code changes.Also, initialized variables to fix the warning "-Werror=maybe-uninitialized".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.I actually wanted to return the error code obtained from fdt_get_mem_rsv() to the caller.In this case, it returns a single error code.I would rather not rely on this.So does this look sane to you ?diff --git a/xen/include/xen/libfdt/libfdt-xen.h b/xen/include/xen/libfdt/libfdt-xen.hindex 3296a368a6..1da87d6668 100644 --- a/xen/include/xen/libfdt/libfdt-xen.h +++ b/xen/include/xen/libfdt/libfdt-xen.h@@ -22,9 +22,8 @@ static inline int fdt_get_mem_rsv_paddr(const void *fdt, int n,uint64_t dt_size; int ret = 0; - ret = fdt_get_mem_rsv(fdt, n, &dt_addr, &dt_size); - if ( ret ) - return ret; + if ( fdt_get_mem_rsv(fdt, n, &dt_addr, &dt_size) < 0 ) + return -FDT_ERR_BADOFFSET;So the problem if you check for ret to be non-zero. But the caller of fdt_get_mem_rsv_paddr() check for < 0.Given that fdt_get_mem_rsv() is not inline, the compiler doesn't know that it will not return a positive value (other than 0). Hence why I think you get an unitialize value.The snippet below should work: if ( ret < 0 ) return ret; Awesome, thanks for the explanation. This works. :) - Ayan Cheers,
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |