Re: [PATCH v5 1/1] libfdt: add fdt_append_addrrange()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



Hi David,

On Tue, Apr 09, 2019 at 04:09:57PM +1000, David Gibson wrote:
> On Tue, Apr 09, 2019 at 03:04:28PM +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>
> 
> Oh, sorry, I wasn't clear.  I already applied the previous version of
> the patch, fixing up the testcase as I went.  If you want further

Just great!

> changes, you'll need to diff them against the new master.

No, I haven't had any change except the said test case.

Thanks,
-Takahiro Akashi

> > ---
> >  libfdt/fdt_addresses.c       | 47 ++++++++++++++++++
> >  libfdt/libfdt.h              | 61 +++++++++++++++++++++++
> >  tests/.gitignore             |  1 +
> >  tests/Makefile.tests         |  1 +
> >  tests/appendprop_addrrange.c | 95 ++++++++++++++++++++++++++++++++++++
> >  tests/run_tests.sh           |  5 ++
> >  tests/testdata.h             |  6 +++
> >  tests/tests.h                |  3 ++
> >  tests/testutils.c            | 58 ++++++++++++++++++++++
> >  9 files changed, 277 insertions(+)
> >  create mode 100644 tests/appendprop_addrrange.c
> > 
> > diff --git a/libfdt/fdt_addresses.c b/libfdt/fdt_addresses.c
> > index f13a87dfa068..2cc997ea5012 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) || ((UINT32_MAX + 1 - addr) < size))
> > +			return -FDT_ERR_BADVALUE;
> > +
> > +		fdt32_st(prop, (uint32_t)addr);
> > +	} else if (addr_cells == 2) {
> > +		fdt64_st(prop, 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, (uint32_t)size);
> > +	} else if (size_cells == 2) {
> > +		fdt64_st(prop, 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..e70f5bf7e8ef 100644
> > --- a/libfdt/libfdt.h
> > +++ b/libfdt/libfdt.h
> > @@ -171,6 +171,16 @@ static inline uint32_t fdt32_ld(const fdt32_t *p)
> >  		| bp[3];
> >  }
> >  
> > +static inline void fdt32_st(void *property, uint32_t value)
> > +{
> > +	uint8_t *bp = property;
> > +
> > +	bp[0] = value >> 24;
> > +	bp[1] = (value >> 16) & 0xff;
> > +	bp[2] = (value >> 8) & 0xff;
> > +	bp[3] = value & 0xff;
> > +}
> > +
> >  static inline uint64_t fdt64_ld(const fdt64_t *p)
> >  {
> >  	const uint8_t *bp = (const uint8_t *)p;
> > @@ -185,6 +195,20 @@ static inline uint64_t fdt64_ld(const fdt64_t *p)
> >  		| bp[7];
> >  }
> >  
> > +static inline void fdt64_st(void *property, uint64_t value)
> > +{
> > +	uint8_t *bp = property;
> > +
> > +	bp[0] = value >> 56;
> > +	bp[1] = (value >> 48) & 0xff;
> > +	bp[2] = (value >> 40) & 0xff;
> > +	bp[3] = (value >> 32) & 0xff;
> > +	bp[4] = (value >> 24) & 0xff;
> > +	bp[5] = (value >> 16) & 0xff;
> > +	bp[6] = (value >> 8) & 0xff;
> > +	bp[7] = value & 0xff;
> > +}
> > +
> >  /**********************************************************************/
> >  /* Traversal functions                                                */
> >  /**********************************************************************/
> > @@ -1831,6 +1855,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/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..c0976140298a
> > --- /dev/null
> > +++ b/tests/appendprop_addrrange.c
> > @@ -0,0 +1,95 @@
> > +/*
> > + * 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;
> > +
> > +	/*
> > +	 * Run test:
> > +	 *  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;
> > +	}
> > +
> > +	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





[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux