| Message ID | 20181115055254.2812-7-takahiro.akashi@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: kexec: add kexec_file_load() support | expand |
[moving some DT people to TO:] On Thu, Nov 15, 2018 at 02:52:45PM +0900, AKASHI Takahiro wrote: > Added function, fdt_setprop_reg(), will be used later to handle > kexec-specific property in arm64's kexec_file implementation. > It will possibly be merged into libfdt in the future. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: devicetree@vger.kernel.org > --- > include/linux/libfdt.h | 26 ++++++++++++++++++++ > lib/Makefile | 2 +- > lib/fdt_addresses.c | 56 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 83 insertions(+), 1 deletion(-) > create mode 100644 lib/fdt_addresses.c We need an ack from the DT side before we can merge this series, please. Will > diff --git a/include/linux/libfdt.h b/include/linux/libfdt.h > index 90ed4ebfa692..47c4dc9e135c 100644 > --- a/include/linux/libfdt.h > +++ b/include/linux/libfdt.h > @@ -5,4 +5,30 @@ > #include <linux/libfdt_env.h> > #include "../../scripts/dtc/libfdt/libfdt.h" > > +/** > + * fdt_setprop_reg - add/set a memory region property > + * @fdt: pointer to the device tree blob > + * @nodeoffset: offset of the node to add a property at > + * @name: name of property > + * @addr: physical start address > + * @size: size of region > + * > + * 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_setprop_reg(void *fdt, int nodeoffset, const char *name, > + u64 addr, u64 size); > + > #endif /* _INCLUDE_LIBFDT_H_ */ > diff --git a/lib/Makefile b/lib/Makefile > index db06d1237898..2a96cb05e15d 100644 > --- a/lib/Makefile > +++ b/lib/Makefile > @@ -205,7 +205,7 @@ KASAN_SANITIZE_stackdepot.o := n > KCOV_INSTRUMENT_stackdepot.o := n > > libfdt_files = fdt.o fdt_ro.o fdt_wip.o fdt_rw.o fdt_sw.o fdt_strerror.o \ > - fdt_empty_tree.o > + fdt_empty_tree.o fdt_addresses.o > $(foreach file, $(libfdt_files), \ > $(eval CFLAGS_$(file) = -I$(src)/../scripts/dtc/libfdt)) > lib-$(CONFIG_LIBFDT) += $(libfdt_files) > diff --git a/lib/fdt_addresses.c b/lib/fdt_addresses.c > new file mode 100644 > index 000000000000..97ddd5a5cc10 > --- /dev/null > +++ b/lib/fdt_addresses.c > @@ -0,0 +1,56 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <linux/libfdt_env.h> > +#include <linux/types.h> > +#include "../scripts/dtc/libfdt/fdt_addresses.c" > + > +/* > + * helper functions for arm64 kexec > + * Those functions may be merged into libfdt in the future. > + */ > + > +/* This function assumes that cells is 1 or 2 */ > +static void cpu64_to_fdt_cells(void *buf, u64 val64, int cells) > +{ > + __be32 val32; > + > + while (cells) { > + val32 = cpu_to_fdt32(val64 >> (32 * (--cells))); > + memcpy(buf, &val32, sizeof(val32)); > + buf += sizeof(val32); > + } > +} > + > +int fdt_setprop_reg(void *fdt, int nodeoffset, const char *name, > + u64 addr, u64 size) > +{ > + int addr_cells, size_cells; > + char buf[sizeof(__be32) * 2 * 2]; > + /* assume dt_root_[addr|size]_cells <= 2 */ > + void *prop; > + size_t buf_size; > + > + addr_cells = fdt_address_cells(fdt, 0); > + if (addr_cells < 0) > + return addr_cells; > + size_cells = fdt_size_cells(fdt, 0); > + if (size_cells < 0) > + return size_cells; > + > + /* if *_cells >= 2, cells can hold 64-bit values anyway */ > + if ((addr_cells == 1) && ((addr > U32_MAX) || > + ((addr + size) > U32_MAX))) > + return -FDT_ERR_BADVALUE; > + > + if ((size_cells == 1) && (size > U32_MAX)) > + return -FDT_ERR_BADVALUE; > + > + buf_size = (addr_cells + size_cells) * sizeof(u32); > + prop = buf; > + > + cpu64_to_fdt_cells(prop, addr, addr_cells); > + prop += addr_cells * sizeof(u32); > + > + cpu64_to_fdt_cells(prop, size, size_cells); > + > + return fdt_setprop(fdt, nodeoffset, name, buf, buf_size); > +} > -- > 2.19.0 >
On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > Added function, fdt_setprop_reg(), will be used later to handle > kexec-specific property in arm64's kexec_file implementation. > It will possibly be merged into libfdt in the future. You generally can't modify libfdt files. Any changes will be blown away with the next dtc sync (there's one in -next now). Though here you are creating a new location with fdt code. lib/ is just a shim to the actual libfdt code. Don't put any implementation there. You can add this to drivers/of/fdt_address.c for the short term, but it still needs to go upstream. Otherwise, the implementation looks fine to me. > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: devicetree@vger.kernel.org > --- > include/linux/libfdt.h | 26 ++++++++++++++++++++ > lib/Makefile | 2 +- > lib/fdt_addresses.c | 56 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 83 insertions(+), 1 deletion(-) > create mode 100644 lib/fdt_addresses.c
Hi Rob, Thanks for reviewing this. On Thu, Dec 06, 2018 at 08:47:04AM -0600, Rob Herring wrote: > On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro > <takahiro.akashi@linaro.org> wrote: > > > > Added function, fdt_setprop_reg(), will be used later to handle > > kexec-specific property in arm64's kexec_file implementation. > > It will possibly be merged into libfdt in the future. > > You generally can't modify libfdt files. Any changes will be blown > away with the next dtc sync (there's one in -next now). Though here > you are creating a new location with fdt code. lib/ is just a shim to > the actual libfdt code. Don't put any implementation there. You can > add this to drivers/of/fdt_address.c for the short term, but it still > needs to go upstream. > > Otherwise, the implementation looks fine to me. I agree, but I don't think there's a real need for us to hack drivers/of/fdt_address.c in the meantime -- let's just target upstream and not carry this in the kernel. Akashi -- for now, I'll drop the kdump parts of this series which rely on this helper. The majority of the series is actually independent and can go in as-is. I've pushed out a kexec branch to the arm64 tree for you to take a look at: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kexec Thanks, Will
Hi Akashi, Will, On 06/12/2018 15:54, Will Deacon wrote: > On Thu, Dec 06, 2018 at 08:47:04AM -0600, Rob Herring wrote: >> On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro >> <takahiro.akashi@linaro.org> wrote: >>> >>> Added function, fdt_setprop_reg(), will be used later to handle >>> kexec-specific property in arm64's kexec_file implementation. >>> It will possibly be merged into libfdt in the future. >> >> You generally can't modify libfdt files. Any changes will be blown >> away with the next dtc sync (there's one in -next now). Though here >> you are creating a new location with fdt code. lib/ is just a shim to >> the actual libfdt code. Don't put any implementation there. You can >> add this to drivers/of/fdt_address.c for the short term, but it still >> needs to go upstream. >> >> Otherwise, the implementation looks fine to me. > > I agree, but I don't think there's a real need for us to hack > drivers/of/fdt_address.c in the meantime -- let's just target upstream > and not carry this in the kernel. > > Akashi -- for now, I'll drop the kdump parts of this series which rely > on this helper. The majority of the series is actually independent and > can go in as-is. > > I've pushed out a kexec branch to the arm64 tree for you to take a look > at: > > https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kexec I gave this a quick spin. Without the elfcorehdr/usable-memory-range arm64 needs to explicitly forbid kdump via kexec_file_load. (like powerpc does already). Without this kdump works, but the second kernel overwrites the first as those DT properties are missing. I'll post a patch momentarily, Thanks, James
James, On Fri, Dec 07, 2018 at 10:12:47AM +0000, James Morse wrote: > Hi Akashi, Will, > > On 06/12/2018 15:54, Will Deacon wrote: > > On Thu, Dec 06, 2018 at 08:47:04AM -0600, Rob Herring wrote: > >> On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro > >> <takahiro.akashi@linaro.org> wrote: > >>> > >>> Added function, fdt_setprop_reg(), will be used later to handle > >>> kexec-specific property in arm64's kexec_file implementation. > >>> It will possibly be merged into libfdt in the future. > >> > >> You generally can't modify libfdt files. Any changes will be blown > >> away with the next dtc sync (there's one in -next now). Though here > >> you are creating a new location with fdt code. lib/ is just a shim to > >> the actual libfdt code. Don't put any implementation there. You can > >> add this to drivers/of/fdt_address.c for the short term, but it still > >> needs to go upstream. > >> > >> Otherwise, the implementation looks fine to me. > > > > I agree, but I don't think there's a real need for us to hack > > drivers/of/fdt_address.c in the meantime -- let's just target upstream > > and not carry this in the kernel. > > > > Akashi -- for now, I'll drop the kdump parts of this series which rely > > on this helper. The majority of the series is actually independent and > > can go in as-is. > > > > I've pushed out a kexec branch to the arm64 tree for you to take a look > > at: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kexec > > I gave this a quick spin. Without the elfcorehdr/usable-memory-range arm64 needs > to explicitly forbid kdump via kexec_file_load. (like powerpc does already). Thank you for pointing this out. > Without this kdump works, but the second kernel overwrites the first as those DT > properties are missing. > > I'll post a patch momentarily, Fine, but anyhow I'm going to submit a new version (*without* kdump), I will fix the issue along with others. -Takahiro Akashi > > Thanks, > > James >
Hi Akashi, On 11/12/2018 06:17, AKASHI, Takahiro wrote: > On Fri, Dec 07, 2018 at 10:12:47AM +0000, James Morse wrote: >> On 06/12/2018 15:54, Will Deacon wrote: >>> On Thu, Dec 06, 2018 at 08:47:04AM -0600, Rob Herring wrote: >>>> On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro >>>> <takahiro.akashi@linaro.org> wrote: >>>>> >>>>> Added function, fdt_setprop_reg(), will be used later to handle >>>>> kexec-specific property in arm64's kexec_file implementation. >>>>> It will possibly be merged into libfdt in the future. >>>> >>>> You generally can't modify libfdt files. Any changes will be blown >>>> away with the next dtc sync (there's one in -next now). Though here >>>> you are creating a new location with fdt code. lib/ is just a shim to >>>> the actual libfdt code. Don't put any implementation there. You can >>>> add this to drivers/of/fdt_address.c for the short term, but it still >>>> needs to go upstream. >>>> >>>> Otherwise, the implementation looks fine to me. >>> >>> I agree, but I don't think there's a real need for us to hack >>> drivers/of/fdt_address.c in the meantime -- let's just target upstream >>> and not carry this in the kernel. >>> >>> Akashi -- for now, I'll drop the kdump parts of this series which rely >>> on this helper. The majority of the series is actually independent and >>> can go in as-is. >>> >>> I've pushed out a kexec branch to the arm64 tree for you to take a look >>> at: >>> >>> https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kexec >> >> I gave this a quick spin. Without the elfcorehdr/usable-memory-range arm64 needs >> to explicitly forbid kdump via kexec_file_load. (like powerpc does already). > > Thank you for pointing this out. > >> Without this kdump works, but the second kernel overwrites the first as those DT >> properties are missing. >> >> I'll post a patch momentarily, > > Fine, but anyhow I'm going to submit a new version (*without* kdump), > I will fix the issue along with others. I had a quick look at the arm64 for-next/core branch. Will has queued the non-kdump parts of this series. If you have changes, they need to be against the arm64 tree. Thanks, James
On Tue, Dec 11, 2018 at 10:09:07AM +0000, James Morse wrote: > Hi Akashi, > > On 11/12/2018 06:17, AKASHI, Takahiro wrote: > > On Fri, Dec 07, 2018 at 10:12:47AM +0000, James Morse wrote: > >> On 06/12/2018 15:54, Will Deacon wrote: > >>> On Thu, Dec 06, 2018 at 08:47:04AM -0600, Rob Herring wrote: > >>>> On Wed, Nov 14, 2018 at 11:52 PM AKASHI Takahiro > >>>> <takahiro.akashi@linaro.org> wrote: > >>>>> > >>>>> Added function, fdt_setprop_reg(), will be used later to handle > >>>>> kexec-specific property in arm64's kexec_file implementation. > >>>>> It will possibly be merged into libfdt in the future. > >>>> > >>>> You generally can't modify libfdt files. Any changes will be blown > >>>> away with the next dtc sync (there's one in -next now). Though here > >>>> you are creating a new location with fdt code. lib/ is just a shim to > >>>> the actual libfdt code. Don't put any implementation there. You can > >>>> add this to drivers/of/fdt_address.c for the short term, but it still > >>>> needs to go upstream. > >>>> > >>>> Otherwise, the implementation looks fine to me. > >>> > >>> I agree, but I don't think there's a real need for us to hack > >>> drivers/of/fdt_address.c in the meantime -- let's just target upstream > >>> and not carry this in the kernel. > >>> > >>> Akashi -- for now, I'll drop the kdump parts of this series which rely > >>> on this helper. The majority of the series is actually independent and > >>> can go in as-is. > >>> > >>> I've pushed out a kexec branch to the arm64 tree for you to take a look > >>> at: > >>> > >>> https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kexec > >> > >> I gave this a quick spin. Without the elfcorehdr/usable-memory-range arm64 needs > >> to explicitly forbid kdump via kexec_file_load. (like powerpc does already). > > > > Thank you for pointing this out. > > > >> Without this kdump works, but the second kernel overwrites the first as those DT > >> properties are missing. > >> > >> I'll post a patch momentarily, > > > > Fine, but anyhow I'm going to submit a new version (*without* kdump), > > I will fix the issue along with others. > > I had a quick look at the arm64 for-next/core branch. Will has queued the > non-kdump parts of this series. > > If you have changes, they need to be against the arm64 tree. Okay! -Takahiro Akashi > > Thanks, > > James
diff --git a/include/linux/libfdt.h b/include/linux/libfdt.h index 90ed4ebfa692..47c4dc9e135c 100644 --- a/include/linux/libfdt.h +++ b/include/linux/libfdt.h @@ -5,4 +5,30 @@ #include <linux/libfdt_env.h> #include "../../scripts/dtc/libfdt/libfdt.h" +/** + * fdt_setprop_reg - add/set a memory region property + * @fdt: pointer to the device tree blob + * @nodeoffset: offset of the node to add a property at + * @name: name of property + * @addr: physical start address + * @size: size of region + * + * 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_setprop_reg(void *fdt, int nodeoffset, const char *name, + u64 addr, u64 size); + #endif /* _INCLUDE_LIBFDT_H_ */ diff --git a/lib/Makefile b/lib/Makefile index db06d1237898..2a96cb05e15d 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -205,7 +205,7 @@ KASAN_SANITIZE_stackdepot.o := n KCOV_INSTRUMENT_stackdepot.o := n libfdt_files = fdt.o fdt_ro.o fdt_wip.o fdt_rw.o fdt_sw.o fdt_strerror.o \ - fdt_empty_tree.o + fdt_empty_tree.o fdt_addresses.o $(foreach file, $(libfdt_files), \ $(eval CFLAGS_$(file) = -I$(src)/../scripts/dtc/libfdt)) lib-$(CONFIG_LIBFDT) += $(libfdt_files) diff --git a/lib/fdt_addresses.c b/lib/fdt_addresses.c new file mode 100644 index 000000000000..97ddd5a5cc10 --- /dev/null +++ b/lib/fdt_addresses.c @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/libfdt_env.h> +#include <linux/types.h> +#include "../scripts/dtc/libfdt/fdt_addresses.c" + +/* + * helper functions for arm64 kexec + * Those functions may be merged into libfdt in the future. + */ + +/* This function assumes that cells is 1 or 2 */ +static void cpu64_to_fdt_cells(void *buf, u64 val64, int cells) +{ + __be32 val32; + + while (cells) { + val32 = cpu_to_fdt32(val64 >> (32 * (--cells))); + memcpy(buf, &val32, sizeof(val32)); + buf += sizeof(val32); + } +} + +int fdt_setprop_reg(void *fdt, int nodeoffset, const char *name, + u64 addr, u64 size) +{ + int addr_cells, size_cells; + char buf[sizeof(__be32) * 2 * 2]; + /* assume dt_root_[addr|size]_cells <= 2 */ + void *prop; + size_t buf_size; + + addr_cells = fdt_address_cells(fdt, 0); + if (addr_cells < 0) + return addr_cells; + size_cells = fdt_size_cells(fdt, 0); + if (size_cells < 0) + return size_cells; + + /* if *_cells >= 2, cells can hold 64-bit values anyway */ + if ((addr_cells == 1) && ((addr > U32_MAX) || + ((addr + size) > U32_MAX))) + return -FDT_ERR_BADVALUE; + + if ((size_cells == 1) && (size > U32_MAX)) + return -FDT_ERR_BADVALUE; + + buf_size = (addr_cells + size_cells) * sizeof(u32); + prop = buf; + + cpu64_to_fdt_cells(prop, addr, addr_cells); + prop += addr_cells * sizeof(u32); + + cpu64_to_fdt_cells(prop, size, size_cells); + + return fdt_setprop(fdt, nodeoffset, name, buf, buf_size); +}
Added function, fdt_setprop_reg(), will be used later to handle kexec-specific property in arm64's kexec_file implementation. It will possibly be merged into libfdt in the future. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> Cc: Rob Herring <robh+dt@kernel.org> Cc: Frank Rowand <frowand.list@gmail.com> Cc: devicetree@vger.kernel.org --- include/linux/libfdt.h | 26 ++++++++++++++++++++ lib/Makefile | 2 +- lib/fdt_addresses.c | 56 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 lib/fdt_addresses.c