drivers/of/of_reserved_mem.c | 2 +- include/linux/of_reserved_mem.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
Previously only a pointer to fdt string pool is saved to struct
reserved_mem as region name.
As on some architectures booting FDT will be wiped at later initialisation
stages, this is breaking reserved_mem users.
Copy and save the whole string into struct reserved_mem to avoid
FDT lifecycle problem.
Reported-by: Kevin Wheatfox <enkerewpo@hotmail.com>
Closes: https://lore.kernel.org/loongarch/ME4P282MB1397447C3C094554C7AF2E37B58E2@ME4P282MB1397.AUSP282.PROD.OUTLOOK.COM/
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
drivers/of/of_reserved_mem.c | 2 +-
include/linux/of_reserved_mem.h | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 46e1c3fbc769..22841599cd83 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -70,7 +70,7 @@ static void __init fdt_reserved_mem_save_node(unsigned long node, const char *un
}
rmem->fdt_node = node;
- rmem->name = uname;
+ strscpy(rmem->name, uname, RESERVED_MEM_NAME_LEN);
rmem->base = base;
rmem->size = size;
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
index e338282da652..ed9de36c9cc9 100644
--- a/include/linux/of_reserved_mem.h
+++ b/include/linux/of_reserved_mem.h
@@ -8,8 +8,10 @@
struct of_phandle_args;
struct reserved_mem_ops;
+#define RESERVED_MEM_NAME_LEN 128
+
struct reserved_mem {
- const char *name;
+ char name[RESERVED_MEM_NAME_LEN];
unsigned long fdt_node;
const struct reserved_mem_ops *ops;
phys_addr_t base;
---
base-commit: bb1b0acdcd66e0d8eedee3570d249e076b89ab32
change-id: 20240821-save_resv_name-4f2e2cb8883b
Best regards,
--
Jiaxun Yang <jiaxun.yang@flygoat.com>On Wed, Aug 21, 2024 at 8:51 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
> Previously only a pointer to fdt string pool is saved to struct
> reserved_mem as region name.
>
> As on some architectures booting FDT will be wiped at later initialisation
> stages, this is breaking reserved_mem users.
What architectures? Be specific.
Why is the FDT wiped? It should be preserved and you need it later to
implement kexec.
>
> Copy and save the whole string into struct reserved_mem to avoid
> FDT lifecycle problem.
>
> Reported-by: Kevin Wheatfox <enkerewpo@hotmail.com>
> Closes: https://lore.kernel.org/loongarch/ME4P282MB1397447C3C094554C7AF2E37B58E2@ME4P282MB1397.AUSP282.PROD.OUTLOOK.COM/
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> drivers/of/of_reserved_mem.c | 2 +-
> include/linux/of_reserved_mem.h | 4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 46e1c3fbc769..22841599cd83 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -70,7 +70,7 @@ static void __init fdt_reserved_mem_save_node(unsigned long node, const char *un
> }
>
> rmem->fdt_node = node;
> - rmem->name = uname;
> + strscpy(rmem->name, uname, RESERVED_MEM_NAME_LEN);
> rmem->base = base;
> rmem->size = size;
>
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index e338282da652..ed9de36c9cc9 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -8,8 +8,10 @@
> struct of_phandle_args;
> struct reserved_mem_ops;
>
> +#define RESERVED_MEM_NAME_LEN 128
> +
> struct reserved_mem {
> - const char *name;
> + char name[RESERVED_MEM_NAME_LEN];
> unsigned long fdt_node;
> const struct reserved_mem_ops *ops;
> phys_addr_t base;
>
> ---
> base-commit: bb1b0acdcd66e0d8eedee3570d249e076b89ab32
> change-id: 20240821-save_resv_name-4f2e2cb8883b
>
> Best regards,
> --
> Jiaxun Yang <jiaxun.yang@flygoat.com>
>
On Wed, Aug 21, 2024 at 02:51:02PM +0100, Jiaxun Yang wrote: > Previously only a pointer to fdt string pool is saved to struct > reserved_mem as region name. > > As on some architectures booting FDT will be wiped at later initialisation > stages, this is breaking reserved_mem users. > > Copy and save the whole string into struct reserved_mem to avoid > FDT lifecycle problem. > > Reported-by: Kevin Wheatfox <enkerewpo@hotmail.com> > Closes: https://lore.kernel.org/loongarch/ME4P282MB1397447C3C094554C7AF2E37B58E2@ME4P282MB1397.AUSP282.PROD.OUTLOOK.COM/ I doubt this uses mainline tree... > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> This is a bigger problem and should be solved unified way in multiple places. You cannot just wipe out FDT from the memory, because it is used in all other places. Your report earlier probably used some custom patches allowing this wipe out, but that's the mistake. Fix your wiping out mechanism... Best regards, Krzysztof
On 25/08/2024 09:18, Krzysztof Kozlowski wrote: > On Wed, Aug 21, 2024 at 02:51:02PM +0100, Jiaxun Yang wrote: >> Previously only a pointer to fdt string pool is saved to struct >> reserved_mem as region name. >> >> As on some architectures booting FDT will be wiped at later initialisation >> stages, this is breaking reserved_mem users. >> >> Copy and save the whole string into struct reserved_mem to avoid >> FDT lifecycle problem. >> >> Reported-by: Kevin Wheatfox <enkerewpo@hotmail.com> >> Closes: https://lore.kernel.org/loongarch/ME4P282MB1397447C3C094554C7AF2E37B58E2@ME4P282MB1397.AUSP282.PROD.OUTLOOK.COM/ > > I doubt this uses mainline tree... > >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > > This is a bigger problem and should be solved unified way in multiple > places. You cannot just wipe out FDT from the memory, because it is used > in all other places. > > Your report earlier probably used some custom patches allowing this > wipe out, but that's the mistake. Fix your wiping out mechanism... The commit msg is quite vague on real problem, so one has to go to original report to find that you use built-in dtb, so only the unflatten_and_copy_device_tree() path, while I have impression you just want to drop the copied (in-kernel) FDT. Fix the commit msg to describe the real problem being fixed here. Provide also proper fixes tag. Best regards, Krzysztof
Hi Jiaxun,
kernel test robot noticed the following build warnings:
[auto build test WARNING on bb1b0acdcd66e0d8eedee3570d249e076b89ab32]
url: https://github.com/intel-lab-lkp/linux/commits/Jiaxun-Yang/of_reserved_mem-Save-region-name-string-into-struct-reserved_mem/20240821-215235
base: bb1b0acdcd66e0d8eedee3570d249e076b89ab32
patch link: https://lore.kernel.org/r/20240821-save_resv_name-v1-1-b9c17f103ffb%40flygoat.com
patch subject: [PATCH] of_reserved_mem: Save region name string into struct reserved_mem
config: i386-buildonly-randconfig-002-20240824 (https://download.01.org/0day-ci/archive/20240824/202408241510.Rk14q8Rg-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240824/202408241510.Rk14q8Rg-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408241510.Rk14q8Rg-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/of/of_reserved_mem.c:467:12: warning: address of array 'rmem->name' will always evaluate to 'true' [-Wpointer-bool-conversion]
467 | rmem->name ? rmem->name : "unknown");
| ~~~~~~^~~~ ~
include/linux/printk.h:538:34: note: expanded from macro 'pr_info'
538 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~
include/linux/printk.h:465:60: note: expanded from macro 'printk'
465 | #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~
include/linux/printk.h:437:19: note: expanded from macro 'printk_index_wrap'
437 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~~~~~~~~
1 warning generated.
vim +467 drivers/of/of_reserved_mem.c
ae1add247bf8c2 Mitchel Humpherys 2015-09-15 426
3f0c8206644836 Marek Szyprowski 2014-02-28 427 /**
c8813f7ec01c67 chenqiwu 2020-05-11 428 * fdt_init_reserved_mem() - allocate and init all saved reserved memory regions
3f0c8206644836 Marek Szyprowski 2014-02-28 429 */
3f0c8206644836 Marek Szyprowski 2014-02-28 430 void __init fdt_init_reserved_mem(void)
3f0c8206644836 Marek Szyprowski 2014-02-28 431 {
3f0c8206644836 Marek Szyprowski 2014-02-28 432 int i;
ae1add247bf8c2 Mitchel Humpherys 2015-09-15 433
ae1add247bf8c2 Mitchel Humpherys 2015-09-15 434 /* check for overlapping reserved regions */
ae1add247bf8c2 Mitchel Humpherys 2015-09-15 435 __rmem_check_for_overlap();
ae1add247bf8c2 Mitchel Humpherys 2015-09-15 436
3f0c8206644836 Marek Szyprowski 2014-02-28 437 for (i = 0; i < reserved_mem_count; i++) {
3f0c8206644836 Marek Szyprowski 2014-02-28 438 struct reserved_mem *rmem = &reserved_mem[i];
3f0c8206644836 Marek Szyprowski 2014-02-28 439 unsigned long node = rmem->fdt_node;
3f0c8206644836 Marek Szyprowski 2014-02-28 440 int err = 0;
6f1188b4ac7577 Yue Hu 2020-07-30 441 bool nomap;
3f0c8206644836 Marek Szyprowski 2014-02-28 442
d0b8ed47e83a22 pierre Kuo 2019-02-19 443 nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
9dcfee01930e6c Marek Szyprowski 2014-07-14 444
3f0c8206644836 Marek Szyprowski 2014-02-28 445 if (rmem->size == 0)
3f0c8206644836 Marek Szyprowski 2014-02-28 446 err = __reserved_mem_alloc_size(node, rmem->name,
3f0c8206644836 Marek Szyprowski 2014-02-28 447 &rmem->base, &rmem->size);
d0b8ed47e83a22 pierre Kuo 2019-02-19 448 if (err == 0) {
d0b8ed47e83a22 pierre Kuo 2019-02-19 449 err = __reserved_mem_init_node(rmem);
d0b8ed47e83a22 pierre Kuo 2019-02-19 450 if (err != 0 && err != -ENOENT) {
d0b8ed47e83a22 pierre Kuo 2019-02-19 451 pr_info("node %s compatible matching fail\n",
d0b8ed47e83a22 pierre Kuo 2019-02-19 452 rmem->name);
d0b8ed47e83a22 pierre Kuo 2019-02-19 453 if (nomap)
7b25995f5319ad Dong Aisheng 2021-06-11 454 memblock_clear_nomap(rmem->base, rmem->size);
3c6867a12a224d Dong Aisheng 2021-06-11 455 else
3ecc68349bbab6 Mike Rapoport 2021-11-05 456 memblock_phys_free(rmem->base,
3ecc68349bbab6 Mike Rapoport 2021-11-05 457 rmem->size);
aeb9267eb6b1df Martin Liu 2023-02-10 458 } else {
aeb9267eb6b1df Martin Liu 2023-02-10 459 phys_addr_t end = rmem->base + rmem->size - 1;
aeb9267eb6b1df Martin Liu 2023-02-10 460 bool reusable =
aeb9267eb6b1df Martin Liu 2023-02-10 461 (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;
aeb9267eb6b1df Martin Liu 2023-02-10 462
6ee7afbabcee4d Geert Uytterhoeven 2023-02-16 463 pr_info("%pa..%pa (%lu KiB) %s %s %s\n",
aeb9267eb6b1df Martin Liu 2023-02-10 464 &rmem->base, &end, (unsigned long)(rmem->size / SZ_1K),
aeb9267eb6b1df Martin Liu 2023-02-10 465 nomap ? "nomap" : "map",
aeb9267eb6b1df Martin Liu 2023-02-10 466 reusable ? "reusable" : "non-reusable",
aeb9267eb6b1df Martin Liu 2023-02-10 @467 rmem->name ? rmem->name : "unknown");
d0b8ed47e83a22 pierre Kuo 2019-02-19 468 }
d0b8ed47e83a22 pierre Kuo 2019-02-19 469 }
3f0c8206644836 Marek Szyprowski 2014-02-28 470 }
3f0c8206644836 Marek Szyprowski 2014-02-28 471 }
9dcfee01930e6c Marek Szyprowski 2014-07-14 472
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2026 Red Hat, Inc.