On Thu, Jan 24, 2019 at 07:20:48PM +0900, AKASHI Takahiro wrote:
> This function will append an address range property using parent node's
> "#address-cells" and "#size-cells" properties.
>
> It will be used in implementing kdump with kexec_file_load system call
> at linux kernel for arm64 once it is merged into kernel tree.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@xxxxxxxxxx>
Sorry I've taken so long to look at this.
> ---
> libfdt/fdt_addresses.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> libfdt/libfdt.h | 37 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 80 insertions(+)
>
> diff --git a/libfdt/fdt_addresses.c b/libfdt/fdt_addresses.c
> index f13a87dfa068..6f13a7bbb6a3 100644
> --- a/libfdt/fdt_addresses.c
> +++ b/libfdt/fdt_addresses.c
> @@ -95,3 +95,46 @@ int fdt_size_cells(const void *fdt, int nodeoffset)
> return 1;
> return val;
> }
> +
> +static void cpu64_to_fdt_cells(uint8_t *prop, uint64_t val, int cells)
> +{
> + fdt32_t val32;
> +
> + while (cells) {
> + val32 = cpu_to_fdt32(val >> (32 * (--cells)));
I'm not sure if you're intending this to work for cells > 2 - with the
upper cells being 0-filled. If so, this will result in a (shift >
width of type) which can trip undefined behaviour.
> + memcpy(prop, &val32, sizeof(val32));
It'd be nicer to add fdt{32,64}_st() helpers for this
> + prop += sizeof(val32);
> + }
> +}
> +
> +/* This function assumes that [address|size]_cells is 1 or 2 */
> +int fdt_appendprop_addrrange(void *fdt, int parent, int nodeoffset,
> + const char *name, uint64_t addr, uint64_t size)
> +{
> + int addr_cells, size_cells;
> + uint8_t data[sizeof(fdt64_t) * 2];
This is only safe for cells <= 2, but you don't actually check for
that and error otherwise.
> + addr_cells = fdt_address_cells(fdt, parent);
> + if (addr_cells < 0)
> + return addr_cells;
> + size_cells = fdt_size_cells(fdt, parent);
> + if (size_cells < 0)
> + return size_cells;
> +
> + if ((addr_cells == 1) &&
> + ((addr > UINT32_MAX) || ((addr + size) > UINT32_MAX)))
> + return -FDT_ERR_BADVALUE;
> +
> + if ((size_cells == 1) && (size > UINT32_MAX))
> + return -FDT_ERR_BADVALUE;
Given you already have a test for cells == 1 specifically, it's barely
worth it to have the quasi-general cpu64_to_fdt_cells() helper.
> +
> + if (!name)
> + name = "reg";
Drop this - allowing the user to pass NULL instead of "reg" has no
real benefit and makes the interface more complex.
> +
> + cpu64_to_fdt_cells(data, addr, addr_cells);
> + cpu64_to_fdt_cells(data + addr_cells * sizeof(fdt32_t), size,
> + size_cells);
> +
> + return fdt_appendprop(fdt, nodeoffset, name, data,
> + (addr_cells + size_cells) * sizeof(fdt32_t));
> +}
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index a470d1df6d2a..c25d980c2547 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -1831,6 +1831,43 @@ static inline int fdt_appendprop_cell(void *fdt, int nodeoffset,
> #define fdt_appendprop_string(fdt, nodeoffset, name, str) \
> fdt_appendprop((fdt), (nodeoffset), (name), (str), strlen(str)+1)
>
> +/**
> + * fdt_appendprop_addrrange - append a address range property
> + * @fdt: pointer to the device tree blob
> + * @parent: offset of the parent node
> + * @nodeoffset: offset of the node to add a property at
> + * @name: name of property
> + * @addr: start address of a given range
> + * @size: size of a given range
> + *
> + * fdt_appendprop_addrrange() appends an address range value (start
> + * address and size) to the value of the named property in the given
> + * node, or creates a new property with that value if it does not
> + * already exist.
> + * If "name" is not specified, a default "reg" is used.
> + * Cell sizes are determined by parent's #address-cells and #size-cells.
> + *
> + * This function may insert data into the blob, and will therefore
> + * change the offsets of some existing nodes.
> + *
> + * returns:
> + * 0, on success
> + * -FDT_ERR_BADLAYOUT,
> + * -FDT_ERR_BADMAGIC,
> + * -FDT_ERR_BADNCELLS, if the node has a badly formatted or invalid
> + * #address-cells property
> + * -FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
> + * -FDT_ERR_BADSTATE,
> + * -FDT_ERR_BADSTRUCTURE,
> + * -FDT_ERR_BADVERSION,
> + * -FDT_ERR_BADVALUE, addr or size doesn't fit to respective cells size
> + * -FDT_ERR_NOSPACE, there is insufficient free space in the blob to
> + * contain a new property
> + * -FDT_ERR_TRUNCATED, standard meanings
> + */
> +int fdt_appendprop_addrrange(void *fdt, int parent, int nodeoffset,
> + const char *name, uint64_t addr, uint64_t size);
> +
> /**
> * fdt_delprop - delete a property
> * @fdt: pointer to the device tree blob
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
