[PATCHv2] libfdt: Revert 6dcb8ba4 from upstream libfdt

Patrice CHOTARD patrice.chotard at st.com
Tue Jan 28 09:19:26 CET 2020


Hi Tom

On 1/27/20 6:10 PM, Tom Rini wrote:
> In upstream libfdt, 6dcb8ba4 "libfdt: Add helpers for accessing
> unaligned words" introduced changes to support unaligned reads for ARM
> platforms and 11738cf01f15 "libfdt: Don't use memcpy to handle unaligned
> reads on ARM" improved the performance of these helpers.
>
> In practice however, this only occurs when the user has forced the
> device tree to be placed in memory in a non-aligned way, which in turn
> violates both our rules and the Linux Kernel rules for how things must
> reside in memory to function.
>
> This "in practice" part is important as handling these other cases adds
> visible (1 second or more) delay to boot in what would be considered the
> fast path of the code.
>
> Cc: Patrice CHOTARD <patrice.chotard at st.com>
> Cc: Patrick DELAUNAY <patrick.delaunay at st.com>
> Link: https://www.spinics.net/lists/devicetree-compiler/msg02972.html
> Signed-off-by: Tom Rini <trini at konsulko.com>
> ---
>  lib/libfdt/fdt_ro.c         | 20 ++++++++++----------
>  scripts/dtc/libfdt/fdt_ro.c | 20 ++++++++++----------
>  scripts/dtc/libfdt/libfdt.h | 33 +--------------------------------
>  3 files changed, 21 insertions(+), 52 deletions(-)
>
> diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
> index 560041b603e1..be03aea9eb00 100644
> --- a/lib/libfdt/fdt_ro.c
> +++ b/lib/libfdt/fdt_ro.c
> @@ -183,8 +183,8 @@ int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size)
>  	if (fdt_chk_extra() && !re)
>  		return -FDT_ERR_BADOFFSET;
>  
> -	*address = fdt64_ld(&re->address);
> -	*size = fdt64_ld(&re->size);
> +	*address = fdt64_to_cpu(re->address);
> +	*size = fdt64_to_cpu(re->size);
>  	return 0;
>  }
>  
> @@ -194,7 +194,7 @@ int fdt_num_mem_rsv(const void *fdt)
>  	const struct fdt_reserve_entry *re;
>  
>  	for (i = 0; (re = fdt_mem_rsv(fdt, i)) != NULL; i++) {
> -		if (fdt64_ld(&re->size) == 0)
> +		if (fdt64_to_cpu(re->size) == 0)
>  			return i;
>  	}
>  	return -FDT_ERR_TRUNCATED;
> @@ -372,7 +372,7 @@ static const struct fdt_property *fdt_get_property_by_offset_(const void *fdt,
>  	prop = fdt_offset_ptr_(fdt, offset);
>  
>  	if (lenp)
> -		*lenp = fdt32_ld(&prop->len);
> +		*lenp = fdt32_to_cpu(prop->len);
>  
>  	return prop;
>  }
> @@ -410,7 +410,7 @@ static const struct fdt_property *fdt_get_property_namelen_(const void *fdt,
>  			offset = -FDT_ERR_INTERNAL;
>  			break;
>  		}
> -		if (fdt_string_eq_(fdt, fdt32_ld(&prop->nameoff),
> +		if (fdt_string_eq_(fdt, fdt32_to_cpu(prop->nameoff),
>  				   name, namelen)) {
>  			if (poffset)
>  				*poffset = offset;
> @@ -463,7 +463,7 @@ const void *fdt_getprop_namelen(const void *fdt, int nodeoffset,
>  
>  	/* Handle realignment */
>  	if (fdt_chk_version() && fdt_version(fdt) < 0x10 &&
> -	    (poffset + sizeof(*prop)) % 8 && fdt32_ld(&prop->len) >= 8)
> +	    (poffset + sizeof(*prop)) % 8 && fdt32_to_cpu(prop->len) >= 8)
>  		return prop->data + 4;
>  	return prop->data;
>  }
> @@ -481,7 +481,7 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset,
>  		int namelen;
>  
>  		if (fdt_chk_extra()) {
> -			name = fdt_get_string(fdt, fdt32_ld(&prop->nameoff),
> +			name = fdt_get_string(fdt, fdt32_to_cpu(prop->nameoff),
>  					      &namelen);
>  			if (!name) {
>  				if (lenp)
> @@ -490,13 +490,13 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset,
>  			}
>  			*namep = name;
>  		} else {
> -			*namep = fdt_string(fdt, fdt32_ld(&prop->nameoff));
> +			*namep = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
>  		}
>  	}
>  
>  	/* Handle realignment */
>  	if (fdt_chk_version() && fdt_version(fdt) < 0x10 &&
> -	    (offset + sizeof(*prop)) % 8 && fdt32_ld(&prop->len) >= 8)
> +	    (offset + sizeof(*prop)) % 8 && fdt32_to_cpu(prop->len) >= 8)
>  		return prop->data + 4;
>  	return prop->data;
>  }
> @@ -521,7 +521,7 @@ uint32_t fdt_get_phandle(const void *fdt, int nodeoffset)
>  			return 0;
>  	}
>  
> -	return fdt32_ld(php);
> +	return fdt32_to_cpu(*php);
>  }
>  
>  const char *fdt_get_alias_namelen(const void *fdt,
> diff --git a/scripts/dtc/libfdt/fdt_ro.c b/scripts/dtc/libfdt/fdt_ro.c
> index e398815485d6..d9d52e0d56e6 100644
> --- a/scripts/dtc/libfdt/fdt_ro.c
> +++ b/scripts/dtc/libfdt/fdt_ro.c
> @@ -179,8 +179,8 @@ int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size)
>  	if (fdt_chk_extra() && !re)
>  		return -FDT_ERR_BADOFFSET;
>  
> -	*address = fdt64_ld(&re->address);
> -	*size = fdt64_ld(&re->size);
> +	*address = fdt64_to_cpu(re->address);
> +	*size = fdt64_to_cpu(re->size);
>  	return 0;
>  }
>  
> @@ -190,7 +190,7 @@ int fdt_num_mem_rsv(const void *fdt)
>  	const struct fdt_reserve_entry *re;
>  
>  	for (i = 0; (re = fdt_mem_rsv(fdt, i)) != NULL; i++) {
> -		if (fdt64_ld(&re->size) == 0)
> +		if (fdt64_to_cpu(re->size) == 0)
>  			return i;
>  	}
>  	return -FDT_ERR_TRUNCATED;
> @@ -368,7 +368,7 @@ static const struct fdt_property *fdt_get_property_by_offset_(const void *fdt,
>  	prop = fdt_offset_ptr_(fdt, offset);
>  
>  	if (lenp)
> -		*lenp = fdt32_ld(&prop->len);
> +		*lenp = fdt32_to_cpu(prop->len);
>  
>  	return prop;
>  }
> @@ -406,7 +406,7 @@ static const struct fdt_property *fdt_get_property_namelen_(const void *fdt,
>  			offset = -FDT_ERR_INTERNAL;
>  			break;
>  		}
> -		if (fdt_string_eq_(fdt, fdt32_ld(&prop->nameoff),
> +		if (fdt_string_eq_(fdt, fdt32_to_cpu(prop->nameoff),
>  				   name, namelen)) {
>  			if (poffset)
>  				*poffset = offset;
> @@ -459,7 +459,7 @@ const void *fdt_getprop_namelen(const void *fdt, int nodeoffset,
>  
>  	/* Handle realignment */
>  	if (fdt_chk_version() && fdt_version(fdt) < 0x10 &&
> -	    (poffset + sizeof(*prop)) % 8 && fdt32_ld(&prop->len) >= 8)
> +	    (poffset + sizeof(*prop)) % 8 && fdt32_to_cpu(prop->len) >= 8)
>  		return prop->data + 4;
>  	return prop->data;
>  }
> @@ -477,7 +477,7 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset,
>  		int namelen;
>  
>  		if (fdt_chk_extra()) {
> -			name = fdt_get_string(fdt, fdt32_ld(&prop->nameoff),
> +			name = fdt_get_string(fdt, fdt32_to_cpu(prop->nameoff),
>  					      &namelen);
>  			if (!name) {
>  				if (lenp)
> @@ -486,13 +486,13 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset,
>  			}
>  			*namep = name;
>  		} else {
> -			*namep = fdt_string(fdt, fdt32_ld(&prop->nameoff));
> +			*namep = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
>  		}
>  	}
>  
>  	/* Handle realignment */
>  	if (fdt_chk_version() && fdt_version(fdt) < 0x10 &&
> -	    (offset + sizeof(*prop)) % 8 && fdt32_ld(&prop->len) >= 8)
> +	    (offset + sizeof(*prop)) % 8 && fdt32_to_cpu(prop->len) >= 8)
>  		return prop->data + 4;
>  	return prop->data;
>  }
> @@ -517,7 +517,7 @@ uint32_t fdt_get_phandle(const void *fdt, int nodeoffset)
>  			return 0;
>  	}
>  
> -	return fdt32_ld(php);
> +	return fdt32_to_cpu(*php);
>  }
>  
>  const char *fdt_get_alias_namelen(const void *fdt,
> diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
> index 36fadcdea516..fa63fffe28e9 100644
> --- a/scripts/dtc/libfdt/libfdt.h
> +++ b/scripts/dtc/libfdt/libfdt.h
> @@ -117,23 +117,6 @@ static inline void *fdt_offset_ptr_w(void *fdt, int offset, int checklen)
>  
>  uint32_t fdt_next_tag(const void *fdt, int offset, int *nextoffset);
>  
> -/*
> - * Alignment helpers:
> - *     These helpers access words from a device tree blob.  They're
> - *     built to work even with unaligned pointers on platforms (ike
> - *     ARM) that don't like unaligned loads and stores
> - */
> -
> -static inline uint32_t fdt32_ld(const fdt32_t *p)
> -{
> -	const uint8_t *bp = (const uint8_t *)p;
> -
> -	return ((uint32_t)bp[0] << 24)
> -		| ((uint32_t)bp[1] << 16)
> -		| ((uint32_t)bp[2] << 8)
> -		| bp[3];
> -}
> -
>  static inline void fdt32_st(void *property, uint32_t value)
>  {
>  	uint8_t *bp = (uint8_t *)property;
> @@ -144,20 +127,6 @@ static inline void fdt32_st(void *property, uint32_t value)
>  	bp[3] = value & 0xff;
>  }
>  
> -static inline uint64_t fdt64_ld(const fdt64_t *p)
> -{
> -	const uint8_t *bp = (const uint8_t *)p;
> -
> -	return ((uint64_t)bp[0] << 56)
> -		| ((uint64_t)bp[1] << 48)
> -		| ((uint64_t)bp[2] << 40)
> -		| ((uint64_t)bp[3] << 32)
> -		| ((uint64_t)bp[4] << 24)
> -		| ((uint64_t)bp[5] << 16)
> -		| ((uint64_t)bp[6] << 8)
> -		| bp[7];
> -}
> -
>  static inline void fdt64_st(void *property, uint64_t value)
>  {
>  	uint8_t *bp = (uint8_t *)property;
> @@ -232,7 +201,7 @@ int fdt_next_subnode(const void *fdt, int offset);
>  /* General functions                                                  */
>  /**********************************************************************/
>  #define fdt_get_header(fdt, field) \
> -	(fdt32_ld(&((const struct fdt_header *)(fdt))->field))
> +	(fdt32_to_cpu(((const struct fdt_header *)(fdt))->field))
>  #define fdt_magic(fdt)			(fdt_get_header(fdt, magic))
>  #define fdt_totalsize(fdt)		(fdt_get_header(fdt, totalsize))
>  #define fdt_off_dt_struct(fdt)		(fdt_get_header(fdt, off_dt_struct))

Tested-by: Patrice Chotard <patrice.chotard at st.com>

Boot time is back to "normal".

Thanks for taking care of this issue.

Patrice


More information about the U-Boot mailing list