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:
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.
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. So does this look sane to
you ?
diff --git a/xen/include/xen/libfdt/libfdt-xen.h
b/xen/include/xen/libfdt/libfdt-xen.h
index 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;
if ( dt_addr != (paddr_t)dt_addr )
{
- Ayan
~Michal
