On Fri, Mar 08, 2019 at 02:05:43PM +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>
> ---
> libfdt/fdt_addresses.c | 47 +++++++++++++++
> libfdt/libfdt.h | 37 ++++++++++++
> libfdt/libfdt_internal.h | 9 +++
> tests/.gitignore | 1 +
> tests/Makefile.tests | 1 +
> tests/appendprop_addrrange.c | 108 +++++++++++++++++++++++++++++++++++
> tests/run_tests.sh | 5 ++
> tests/testdata.h | 6 ++
> tests/tests.h | 3 +
> tests/testutils.c | 58 +++++++++++++++++++
> 10 files changed, 275 insertions(+)
> create mode 100644 tests/appendprop_addrrange.c
>
> diff --git a/libfdt/fdt_addresses.c b/libfdt/fdt_addresses.c
> index f13a87dfa068..7a00c6197872 100644
> --- a/libfdt/fdt_addresses.c
> +++ b/libfdt/fdt_addresses.c
> @@ -95,3 +95,50 @@ int fdt_size_cells(const void *fdt, int nodeoffset)
> return 1;
> return val;
> }
> +
> +/* 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, ret;
> + uint8_t data[sizeof(fdt64_t) * 2], *prop;
> +
> + ret = fdt_address_cells(fdt, parent);
> + if (ret < 0)
> + return ret;
> + addr_cells = ret;
> +
> + ret = fdt_size_cells(fdt, parent);
> + if (ret < 0)
> + return ret;
> + size_cells = ret;
> +
> + /* check validity of address */
> + prop = data;
> + if (addr_cells == 1) {
> + if ((addr > UINT32_MAX) || ((addr + size) > UINT32_MAX))
> + return -FDT_ERR_BADVALUE;
This test could fail to detect bad values in the case of 64-bit
overflow - probably best to check for that first.
> + fdt32_st(prop, cpu_to_fdt32((uint32_t)addr));
fdtXX_st() should, like fdtXX_ld() do its own byteswapping.
> + } else if (addr_cells == 2) {
> + fdt64_st(prop, cpu_to_fdt64(addr));
> + } else {
> + return -FDT_ERR_BADNCELLS;
> + }
> +
> + /* check validity of size */
> + prop += addr_cells * sizeof(fdt32_t);
> + if (size_cells == 1) {
> + if (size > UINT32_MAX)
> + return -FDT_ERR_BADVALUE;
> +
> + fdt32_st(prop, cpu_to_fdt32((uint32_t)size));
> + } else if (size_cells == 2) {
> + fdt64_st(prop, cpu_to_fdt64(size));
> + } else {
> + return -FDT_ERR_BADNCELLS;
> + }
> +
> + 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
> diff --git a/libfdt/libfdt_internal.h b/libfdt/libfdt_internal.h
> index 4109f890ae60..f1ce1d84f7f0 100644
> --- a/libfdt/libfdt_internal.h
> +++ b/libfdt/libfdt_internal.h
> @@ -93,4 +93,13 @@ static inline struct fdt_reserve_entry *fdt_mem_rsv_w_(void *fdt, int n)
>
> #define FDT_SW_MAGIC (~FDT_MAGIC)
>
> +static inline void *fdt32_st(void *property, fdt32_t value)
> +{
> + return memcpy(property, &value, sizeof(value));
First, as above, this should do its own byteswapping (because it knows
the data size).
Second, I've been told that a memcpy() like this can be surprisingly
expensive on certain ARM chips, so doing it with byte accesses and
shift arithmetic like fdtXX_ld() would be preferable.
> +}
> +
> +static inline void *fdt64_st(void *property, fdt64_t value)
> +{
> + return memcpy(property, &value, sizeof(value));
> +}
> #endif /* LIBFDT_INTERNAL_H */
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 12af43868e09..0bc78aa5842b 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -8,6 +8,7 @@ tmp.*
> /addr_size_cells
> /addr_size_cells2
> /appendprop[12]
> +/appendprop_addrrange
> /asm_tree_dump
> /boot-cpuid
> /char_literal
> diff --git a/tests/Makefile.tests b/tests/Makefile.tests
> index b02d8bf3d15b..b77f121f0ba6 100644
> --- a/tests/Makefile.tests
> +++ b/tests/Makefile.tests
> @@ -10,6 +10,7 @@ LIB_TESTS_L = get_mem_rsv \
> notfound \
> addr_size_cells \
> addr_size_cells2 \
> + appendprop_addrrange \
> stringlist \
> setprop_inplace nop_property nop_node \
> sw_tree1 sw_states \
> diff --git a/tests/appendprop_addrrange.c b/tests/appendprop_addrrange.c
> new file mode 100644
> index 000000000000..b54f9049ff6a
> --- /dev/null
> +++ b/tests/appendprop_addrrange.c
> @@ -0,0 +1,108 @@
> +/*
> + * libfdt - Flat Device Tree manipulation
> + * Testcase for fdt_appendprop_addrrange()
> + * Copyright (C) 2018 AKASHI Takahiro, Linaro Limited
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either version 2.1 of
> + * the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdint.h>
> +
> +#include <libfdt.h>
> +
> +#include "tests.h"
> +#include "testdata.h"
> +
> +int main(int argc, char *argv[])
> +{
> + void *fdt, *buf;
> + int offset, xac, xsc, num, i, err;
> + uint64_t addr, size;
> +
> + if (argc != 5)
> + CONFIG("Usage: %s <dtb file> <address-cells> <size-cells> <num>\n",
> + argv[0]);
> +
> + test_init(argc, argv);
> + fdt = load_blob(argv[1]);
> + xac = strtol(argv[2], NULL, 10);
> + xsc = strtol(argv[3], NULL, 10);
> + num = strtol(argv[4], NULL, 10);
> +
> + buf = xmalloc(0x1000);
> + if (!buf)
> + FAIL("Couldn't allocate temporary buffer");
> + err = fdt_open_into(fdt, buf, 0x1000);
> + if (err)
> + FAIL("fdt_open_into(): %s", fdt_strerror(err));
> +
> + fdt = buf;
> +
> + /* Set up */
> + err = fdt_setprop_cell(fdt, 0, "#address-cells", xac);
> + if (err)
> + FAIL("fdt_setprop_cell(\"#address-cells\"): %s",
> + fdt_strerror(err));
> + err = fdt_setprop_cell(fdt, 0, "#size-cells", xsc);
> + if (err)
> + FAIL("fdt_setprop_cell(\"#size-cells\"): %s",
> + fdt_strerror(err));
> +
> + offset = fdt_path_offset(fdt, "/node@1");
> + if (offset < 0)
> + FAIL("Couldn't find path %s", "/node@1");
> +
> + addr = TEST_MEMREGION_ADDR;
> + if (xac > 1)
> + addr += TEST_MEMREGION_ADDR_HI;
> + size = TEST_MEMREGION_SIZE;
> + if (xsc > 1)
> + size += TEST_MEMREGION_SIZE_HI;
> +
> + /*
> + * Do test
> + */
> + /* 1. repeat append's */
> + for (i = 0; i < num; i++) {
> + err = fdt_appendprop_addrrange(fdt, 0, offset,
> + "prop-memregion", addr, size);
> + if (err)
> + FAIL("Failed to append[%d] \"prop-memregion\": %s",
> + i, fdt_strerror(err));
> +
> + check_getprop_addrrange(fdt, 0, offset, "prop-memregion",
> + i + 1);
> +
> + addr += size;
> + size += TEST_MEMREGION_SIZE_INC;
> + }
> +
> + /* 2. default property name */
> + addr = TEST_MEMREGION_ADDR;
> + if (xac > 1)
> + addr += TEST_MEMREGION_ADDR_HI;
> + size = TEST_MEMREGION_SIZE;
> + if (xsc > 1)
> + size += TEST_MEMREGION_SIZE_HI;
> +
> + err = fdt_appendprop_addrrange(fdt, 0, offset, NULL, addr, size);
> + if (err)
> + FAIL("Failed to set \"reg\"(default): %s", fdt_strerror(err));
> + check_getprop_addrrange(fdt, 0, offset, "reg", 1);
> +
> + PASS();
> +}
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 04fc1e8a2bf7..f6b308f045e2 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -424,6 +424,11 @@ libfdt_tests () {
> run_dtc_test -I dts -O dtb -o property_iterate.dtb property_iterate.dts
> run_test property_iterate property_iterate.dtb
>
> + run_dtc_test -I dts -O dtb -o unit-addr-without-reg.dtb unit-addr-without-reg.dts
> + run_test appendprop_addrrange unit-addr-without-reg.dtb 1 1 1
> + run_test appendprop_addrrange unit-addr-without-reg.dtb 2 2 2
> + run_test appendprop_addrrange unit-addr-without-reg.dtb 2 1 3
> +
> # Tests for behaviour on various sorts of corrupted trees
> run_test truncated_property
> run_test truncated_string
> diff --git a/tests/testdata.h b/tests/testdata.h
> index 68dcbaceada4..0d08efb20d52 100644
> --- a/tests/testdata.h
> +++ b/tests/testdata.h
> @@ -40,6 +40,12 @@
> #define TEST_CHAR4 '\''
> #define TEST_CHAR5 '\xff'
>
> +#define TEST_MEMREGION_ADDR 0x12345678
> +#define TEST_MEMREGION_ADDR_HI 0x8765432100000000
> +#define TEST_MEMREGION_SIZE 0x9abcdef0
> +#define TEST_MEMREGION_SIZE_HI 0x0fedcba900000000
> +#define TEST_MEMREGION_SIZE_INC 0x1000
> +
> #ifndef __ASSEMBLY__
> extern struct fdt_header test_tree1;
> extern struct fdt_header truncated_property;
> diff --git a/tests/tests.h b/tests/tests.h
> index dc8120eb6d9d..75735d63ee4c 100644
> --- a/tests/tests.h
> +++ b/tests/tests.h
> @@ -128,6 +128,9 @@ const void *check_get_prop_offset(void *fdt, int poffset, const char *in_name,
> check_get_prop_offset(fdt, poffset, name, sizeof(x), &x); \
> })
>
> +const void *check_getprop_addrrange(void *fdt, int parent, int nodeoffset,
> + const char *name, int num);
> +
> int nodename_eq(const char *s1, const char *s2);
> void vg_prepare_blob(void *fdt, size_t bufsize);
> void *load_blob(const char *filename);
> diff --git a/tests/testutils.c b/tests/testutils.c
> index bbfda90b9cdd..a250d5a5d7f9 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -45,6 +45,7 @@ static inline void VALGRIND_MAKE_MEM_DEFINED(void *p, size_t len)
> #include <libfdt.h>
>
> #include "tests.h"
> +#include "testdata.h"
>
> /* For FDT_SW_MAGIC */
> #include "libfdt_internal.h"
> @@ -184,6 +185,63 @@ const void *check_get_prop_offset(void *fdt, int poffset, const char *exp_name,
> return propval;
> }
>
> +const void *check_getprop_addrrange(void *fdt, int parent, int nodeoffset,
> + const char *name, int num)
> +{
> + const void *propval;
> + int xac, xsc, buf_size, cells, i;
> + char *buf, *p;
> + uint64_t addr, size;
> + fdt32_t val;
> +
> + xac = fdt_address_cells(fdt, parent);
> + xsc = fdt_size_cells(fdt, parent);
> +
> + if (xac <= 0)
> + FAIL("Couldn't identify #address-cells: %s",
> + fdt_strerror(xac));
> + if (xsc <= 0)
> + FAIL("Couldn't identify #size-cells: %s",
> + fdt_strerror(xsc));
> +
> + buf_size = (xac + xsc) * sizeof(fdt32_t) * num;
> + buf = malloc(buf_size);
> + if (!buf)
> + FAIL("Couldn't allocate temporary buffer");
> +
> + /* expected value */
> + addr = TEST_MEMREGION_ADDR;
> + if (xac > 1)
> + addr += TEST_MEMREGION_ADDR_HI;
> + size = TEST_MEMREGION_SIZE;
> + if (xsc > 1)
> + size += TEST_MEMREGION_SIZE_HI;
> + for (p = buf, i = 0; i < num; i++) {
> + cells = xac;
> + while (cells) {
> + val = cpu_to_fdt32(addr >> (32 * (--cells)));
> + memcpy(p, &val, sizeof(val));
> + p += sizeof(val);
> + }
> + cells = xsc;
> + while (cells) {
> + val = cpu_to_fdt32(size >> (32 * (--cells)));
> + memcpy(p, &val, sizeof(val));
> + p += sizeof(val);
> + }
> +
> + addr += size;
> + size += TEST_MEMREGION_SIZE_INC;
> + }
> +
> + /* check */
> + propval = check_getprop(fdt, nodeoffset, name, buf_size,
> + (const void *)buf);
> +
> + free(buf);
> +
> + return propval;
> +}
>
> int nodename_eq(const char *s1, const char *s2)
> {
--
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
