LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan()
@ 2019-05-19 16:04 Hsin-Yi Wang
  2019-05-19 16:04 ` [PATCH v4 2/3] fdt: add support for rng-seed Hsin-Yi Wang
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Hsin-Yi Wang @ 2019-05-19 16:04 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Rob Herring, devicetree, linux-kernel, Frank Rowand,
	Catalin Marinas, Will Deacon, Andrew Morton, Mike Rapoport,
	Ard Biesheuvel, Miles Chen, Hsin-Yi Wang, James Morse,
	Andrew Murray, Mark Rutland, Jun Yao, Yu Zhao, Robin Murphy,
	Laura Abbott, Stephen Boyd, Kees Cook

Currently in arm64, FDT is mapped to RO before it's passed to
early_init_dt_scan(). However, there might be some code that needs
to modify FDT during init. Map FDT to RW until unflatten DT.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
change log v2->v4:
* v3 abandoned
* add an arg pgprot_t to fixmap_remap_fdt()
---
 arch/arm64/include/asm/mmu.h | 2 +-
 arch/arm64/kernel/setup.c    | 5 ++++-
 arch/arm64/mm/mmu.c          | 4 ++--
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index 67ef25d037ea..4499cb00ece7 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -137,7 +137,7 @@ extern void init_mem_pgprot(void);
 extern void create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
 			       unsigned long virt, phys_addr_t size,
 			       pgprot_t prot, bool page_mappings_only);
-extern void *fixmap_remap_fdt(phys_addr_t dt_phys);
+extern void *fixmap_remap_fdt(phys_addr_t dt_phys, pgprot_t prot);
 extern void mark_linear_text_alias_ro(void);
 
 #define INIT_MM_CONTEXT(name)	\
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 413d566405d1..064df3de1d14 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -181,7 +181,7 @@ static void __init smp_build_mpidr_hash(void)
 
 static void __init setup_machine_fdt(phys_addr_t dt_phys)
 {
-	void *dt_virt = fixmap_remap_fdt(dt_phys);
+	void *dt_virt = fixmap_remap_fdt(dt_phys, PAGE_KERNEL);
 	const char *name;
 
 	if (!dt_virt || !early_init_dt_scan(dt_virt)) {
@@ -320,6 +320,9 @@ void __init setup_arch(char **cmdline_p)
 	/* Parse the ACPI tables for possible boot-time configuration */
 	acpi_boot_table_init();
 
+	/* remap fdt to RO */
+	fixmap_remap_fdt(__fdt_pointer, PAGE_KERNEL_RO);
+
 	if (acpi_disabled)
 		unflatten_device_tree();
 
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index a170c6369a68..29648e86f7e5 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -940,12 +940,12 @@ void *__init __fixmap_remap_fdt(phys_addr_t dt_phys, int *size, pgprot_t prot)
 	return dt_virt;
 }
 
-void *__init fixmap_remap_fdt(phys_addr_t dt_phys)
+void *__init fixmap_remap_fdt(phys_addr_t dt_phys, pgprot_t prot)
 {
 	void *dt_virt;
 	int size;
 
-	dt_virt = __fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL_RO);
+	dt_virt = __fixmap_remap_fdt(dt_phys, &size, prot);
 	if (!dt_virt)
 		return NULL;
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 2/3] fdt: add support for rng-seed
  2019-05-19 16:04 [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan() Hsin-Yi Wang
@ 2019-05-19 16:04 ` Hsin-Yi Wang
  2019-05-19 23:54   ` Nicolas Boichat
  2019-05-19 16:04 ` [PATCH v4 3/3] arm64: kexec_file: add rng-seed support Hsin-Yi Wang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Hsin-Yi Wang @ 2019-05-19 16:04 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Rob Herring, devicetree, linux-kernel, Frank Rowand,
	Catalin Marinas, Will Deacon, Andrew Morton, Mike Rapoport,
	Ard Biesheuvel, Miles Chen, Hsin-Yi Wang, James Morse,
	Andrew Murray, Mark Rutland, Jun Yao, Yu Zhao, Robin Murphy,
	Laura Abbott, Stephen Boyd, Kees Cook

Introducing a chosen node, rng-seed, which is an entropy that can be
passed to kernel called very early to increase initial device
randomness. Bootloader should provide this entropy and the value is
read from /chosen/rng-seed in DT.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
change log v2->v4:
* v3 abandoned
* fix doc error
---
 Documentation/devicetree/bindings/chosen.txt | 14 ++++++++++++++
 drivers/of/fdt.c                             | 10 ++++++++++
 2 files changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
index 45e79172a646..678e81bc4383 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -28,6 +28,20 @@ mode) when EFI_RNG_PROTOCOL is supported, it will be overwritten by
 the Linux EFI stub (which will populate the property itself, using
 EFI_RNG_PROTOCOL).
 
+rng-seed
+-----------
+
+This property serves as an entropy to add device randomness. It is parsed
+as a byte array, e.g.
+
+/ {
+	chosen {
+		rng-seed = <0x31 0x95 0x1b 0x3c 0xc9 0xfa 0xb3 ...>;
+	};
+};
+
+This random value should be provided by bootloader.
+
 stdout-path
 -----------
 
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index de893c9616a1..7f3d72921b23 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -24,6 +24,7 @@
 #include <linux/debugfs.h>
 #include <linux/serial_core.h>
 #include <linux/sysfs.h>
+#include <linux/random.h>
 
 #include <asm/setup.h>  /* for COMMAND_LINE_SIZE */
 #include <asm/page.h>
@@ -1079,6 +1080,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 {
 	int l;
 	const char *p;
+	const void *rng_seed;
 
 	pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
 
@@ -1113,6 +1115,14 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 
 	pr_debug("Command line is: %s\n", (char*)data);
 
+	rng_seed = of_get_flat_dt_prop(node, "rng-seed", &l);
+	if (rng_seed && l > 0) {
+		add_device_randomness(rng_seed, l);
+
+		/* try to clear seed so it won't be found. */
+		fdt_delprop(initial_boot_params, node, "rng-seed");
+	}
+
 	/* break now */
 	return 1;
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 3/3] arm64: kexec_file: add rng-seed support
  2019-05-19 16:04 [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan() Hsin-Yi Wang
  2019-05-19 16:04 ` [PATCH v4 2/3] fdt: add support for rng-seed Hsin-Yi Wang
@ 2019-05-19 16:04 ` Hsin-Yi Wang
  2019-05-19 23:57 ` [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan() Nicolas Boichat
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Hsin-Yi Wang @ 2019-05-19 16:04 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Rob Herring, devicetree, linux-kernel, Frank Rowand,
	Catalin Marinas, Will Deacon, Andrew Morton, Mike Rapoport,
	Ard Biesheuvel, Miles Chen, Hsin-Yi Wang, James Morse,
	Andrew Murray, Mark Rutland, Jun Yao, Yu Zhao, Robin Murphy,
	Laura Abbott, Stephen Boyd, Kees Cook

Adding "rng-seed" to dtb. It's fine to add this property if original
fdt doesn't contain it. Since original seed will be deleted after
read, so use a default size 128 bytes here.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
* Will add corresponding part to userspace kexec-tools if this is accepted.
---
 arch/arm64/kernel/machine_kexec_file.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index 58871333737a..d40fde72a023 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -27,6 +27,8 @@
 #define FDT_PROP_INITRD_END	"linux,initrd-end"
 #define FDT_PROP_BOOTARGS	"bootargs"
 #define FDT_PROP_KASLR_SEED	"kaslr-seed"
+#define FDT_PROP_RNG_SEED	"rng-seed"
+#define RNG_SEED_SIZE		128
 
 const struct kexec_file_ops * const kexec_file_loaders[] = {
 	&kexec_image_ops,
@@ -102,6 +104,23 @@ static int setup_dtb(struct kimage *image,
 				FDT_PROP_KASLR_SEED);
 	}
 
+	/* add rng-seed */
+	if (rng_is_initialized()) {
+		void *rng_seed = kmalloc(RNG_SEED_SIZE, GFP_ATOMIC);
+		get_random_bytes(rng_seed, RNG_SEED_SIZE);
+
+		ret = fdt_setprop(dtb, off, FDT_PROP_RNG_SEED, rng_seed,
+				RNG_SEED_SIZE);
+		kfree(rng_seed);
+
+		if (ret)
+			goto out;
+
+	} else {
+		pr_notice("RNG is not initialised: omitting \"%s\" property\n",
+				FDT_PROP_RNG_SEED);
+	}
+
 out:
 	if (ret)
 		return (ret == -FDT_ERR_NOSPACE) ? -ENOMEM : -EINVAL;
@@ -110,7 +129,8 @@ static int setup_dtb(struct kimage *image,
 }
 
 /*
- * More space needed so that we can add initrd, bootargs and kaslr-seed.
+ * More space needed so that we can add initrd, bootargs, kaslr-seed, and
+ * rng-seed.
  */
 #define DTB_EXTRA_SPACE 0x1000
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/3] fdt: add support for rng-seed
  2019-05-19 16:04 ` [PATCH v4 2/3] fdt: add support for rng-seed Hsin-Yi Wang
@ 2019-05-19 23:54   ` Nicolas Boichat
  2019-05-21  4:09     ` Hsin-Yi Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Boichat @ 2019-05-19 23:54 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: linux-arm Mailing List, Rob Herring, devicetree, lkml,
	Frank Rowand, Catalin Marinas, Will Deacon, Andrew Morton,
	Mike Rapoport, Ard Biesheuvel, Miles Chen, James Morse,
	Andrew Murray, Mark Rutland, Jun Yao, Yu Zhao, Robin Murphy,
	Laura Abbott, Stephen Boyd, Kees Cook

On Mon, May 20, 2019 at 1:09 AM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
>
> Introducing a chosen node, rng-seed, which is an entropy that can be
> passed to kernel called very early to increase initial device
> randomness. Bootloader should provide this entropy and the value is
> read from /chosen/rng-seed in DT.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> change log v2->v4:
> * v3 abandoned
> * fix doc error
> ---
>  Documentation/devicetree/bindings/chosen.txt | 14 ++++++++++++++
>  drivers/of/fdt.c                             | 10 ++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
> index 45e79172a646..678e81bc4383 100644
> --- a/Documentation/devicetree/bindings/chosen.txt
> +++ b/Documentation/devicetree/bindings/chosen.txt
> @@ -28,6 +28,20 @@ mode) when EFI_RNG_PROTOCOL is supported, it will be overwritten by
>  the Linux EFI stub (which will populate the property itself, using
>  EFI_RNG_PROTOCOL).
>
> +rng-seed
> +-----------
> +
> +This property serves as an entropy to add device randomness. It is parsed
> +as a byte array, e.g.
> +
> +/ {
> +       chosen {
> +               rng-seed = <0x31 0x95 0x1b 0x3c 0xc9 0xfa 0xb3 ...>;
> +       };
> +};
> +
> +This random value should be provided by bootloader.
> +
>  stdout-path
>  -----------
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index de893c9616a1..7f3d72921b23 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -24,6 +24,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/serial_core.h>
>  #include <linux/sysfs.h>
> +#include <linux/random.h>

Alphabetical order.

>
>  #include <asm/setup.h>  /* for COMMAND_LINE_SIZE */
>  #include <asm/page.h>
> @@ -1079,6 +1080,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>  {
>         int l;
>         const char *p;
> +       const void *rng_seed;
>
>         pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
>
> @@ -1113,6 +1115,14 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>
>         pr_debug("Command line is: %s\n", (char*)data);
>
> +       rng_seed = of_get_flat_dt_prop(node, "rng-seed", &l);
> +       if (rng_seed && l > 0) {
> +               add_device_randomness(rng_seed, l);
> +
> +               /* try to clear seed so it won't be found. */
> +               fdt_delprop(initial_boot_params, node, "rng-seed");

I'm a little bit concerned about this, as we really want the rng-seed
value to be wiped, and not kept in memory (even if it's hard to
access).

IIUC, fdt_delprop splices the device tree, so it'll override
"rng-seed" property with whatever device tree entries follow it.
However, if rng-seed is the last property (or if the entries that
follow are smaller than rng-seed), the seed will stay in memory (or
part of it).

fdt_nop_property in v2 would erase it for sure. I don't know if there
is a way to make sure that rng-seed is removed for good while still
deleting the property (maybe modify fdt_splice_ to do a memset(.., 0)
of the moved chunk?).


> +       }
> +
>         /* break now */
>         return 1;
>  }
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan()
  2019-05-19 16:04 [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan() Hsin-Yi Wang
  2019-05-19 16:04 ` [PATCH v4 2/3] fdt: add support for rng-seed Hsin-Yi Wang
  2019-05-19 16:04 ` [PATCH v4 3/3] arm64: kexec_file: add rng-seed support Hsin-Yi Wang
@ 2019-05-19 23:57 ` Nicolas Boichat
  2019-05-24  0:04 ` Stephen Boyd
  2019-05-24 21:58 ` Rob Herring
  4 siblings, 0 replies; 10+ messages in thread
From: Nicolas Boichat @ 2019-05-19 23:57 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: linux-arm Mailing List, Rob Herring, devicetree, lkml,
	Frank Rowand, Catalin Marinas, Will Deacon, Andrew Morton,
	Mike Rapoport, Ard Biesheuvel, Miles Chen, James Morse,
	Andrew Murray, Mark Rutland, Jun Yao, Yu Zhao, Robin Murphy,
	Laura Abbott, Stephen Boyd, Kees Cook

s/amr64/arm64/ in the commit title.

On Mon, May 20, 2019 at 1:09 AM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
>
> Currently in arm64, FDT is mapped to RO before it's passed to
> early_init_dt_scan(). However, there might be some code that needs
> to modify FDT during init.

I'd give a specific example (i.e. mention the next commit that
introduces rng-seed).

> Map FDT to RW until unflatten DT.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> change log v2->v4:
> * v3 abandoned
> * add an arg pgprot_t to fixmap_remap_fdt()
> ---
>  arch/arm64/include/asm/mmu.h | 2 +-
>  arch/arm64/kernel/setup.c    | 5 ++++-
>  arch/arm64/mm/mmu.c          | 4 ++--
>  3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 67ef25d037ea..4499cb00ece7 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -137,7 +137,7 @@ extern void init_mem_pgprot(void);
>  extern void create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
>                                unsigned long virt, phys_addr_t size,
>                                pgprot_t prot, bool page_mappings_only);
> -extern void *fixmap_remap_fdt(phys_addr_t dt_phys);
> +extern void *fixmap_remap_fdt(phys_addr_t dt_phys, pgprot_t prot);
>  extern void mark_linear_text_alias_ro(void);
>
>  #define INIT_MM_CONTEXT(name)  \
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 413d566405d1..064df3de1d14 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -181,7 +181,7 @@ static void __init smp_build_mpidr_hash(void)
>
>  static void __init setup_machine_fdt(phys_addr_t dt_phys)
>  {
> -       void *dt_virt = fixmap_remap_fdt(dt_phys);
> +       void *dt_virt = fixmap_remap_fdt(dt_phys, PAGE_KERNEL);
>         const char *name;
>
>         if (!dt_virt || !early_init_dt_scan(dt_virt)) {
> @@ -320,6 +320,9 @@ void __init setup_arch(char **cmdline_p)
>         /* Parse the ACPI tables for possible boot-time configuration */
>         acpi_boot_table_init();
>
> +       /* remap fdt to RO */
> +       fixmap_remap_fdt(__fdt_pointer, PAGE_KERNEL_RO);
> +
>         if (acpi_disabled)
>                 unflatten_device_tree();
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index a170c6369a68..29648e86f7e5 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -940,12 +940,12 @@ void *__init __fixmap_remap_fdt(phys_addr_t dt_phys, int *size, pgprot_t prot)
>         return dt_virt;
>  }
>
> -void *__init fixmap_remap_fdt(phys_addr_t dt_phys)
> +void *__init fixmap_remap_fdt(phys_addr_t dt_phys, pgprot_t prot)
>  {
>         void *dt_virt;
>         int size;
>
> -       dt_virt = __fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL_RO);
> +       dt_virt = __fixmap_remap_fdt(dt_phys, &size, prot);
>         if (!dt_virt)
>                 return NULL;
>
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/3] fdt: add support for rng-seed
  2019-05-19 23:54   ` Nicolas Boichat
@ 2019-05-21  4:09     ` Hsin-Yi Wang
  2019-05-21 12:42       ` Nicolas Boichat
  0 siblings, 1 reply; 10+ messages in thread
From: Hsin-Yi Wang @ 2019-05-21  4:09 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: linux-arm Mailing List, Rob Herring, devicetree, lkml,
	Frank Rowand, Catalin Marinas, Will Deacon, Andrew Morton,
	Mike Rapoport, Ard Biesheuvel, Miles Chen, James Morse,
	Andrew Murray, Mark Rutland, Jun Yao, Yu Zhao, Robin Murphy,
	Laura Abbott, Stephen Boyd, Kees Cook

On Mon, May 20, 2019 at 7:54 AM Nicolas Boichat <drinkcat@chromium.org> wrote:

> Alphabetical order.
Original headers are not sorted, should I sort them here?
>

>
> I'm a little bit concerned about this, as we really want the rng-seed
> value to be wiped, and not kept in memory (even if it's hard to
> access).
>
> IIUC, fdt_delprop splices the device tree, so it'll override
> "rng-seed" property with whatever device tree entries follow it.
> However, if rng-seed is the last property (or if the entries that
> follow are smaller than rng-seed), the seed will stay in memory (or
> part of it).
>
> fdt_nop_property in v2 would erase it for sure. I don't know if there
> is a way to make sure that rng-seed is removed for good while still
> deleting the property (maybe modify fdt_splice_ to do a memset(.., 0)
> of the moved chunk?).
>
So maybe we can use fdt_nop_property() back?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/3] fdt: add support for rng-seed
  2019-05-21  4:09     ` Hsin-Yi Wang
@ 2019-05-21 12:42       ` Nicolas Boichat
  0 siblings, 0 replies; 10+ messages in thread
From: Nicolas Boichat @ 2019-05-21 12:42 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: linux-arm Mailing List, Rob Herring, devicetree, lkml,
	Frank Rowand, Catalin Marinas, Will Deacon, Andrew Morton,
	Mike Rapoport, Ard Biesheuvel, Miles Chen, James Morse,
	Andrew Murray, Mark Rutland, Jun Yao, Yu Zhao, Robin Murphy,
	Laura Abbott, Stephen Boyd, Kees Cook

On Tue, May 21, 2019 at 12:10 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
>
> On Mon, May 20, 2019 at 7:54 AM Nicolas Boichat <drinkcat@chromium.org> wrote:
>
> > Alphabetical order.
> Original headers are not sorted, should I sort them here?
> >
>
> >
> > I'm a little bit concerned about this, as we really want the rng-seed
> > value to be wiped, and not kept in memory (even if it's hard to
> > access).
> >
> > IIUC, fdt_delprop splices the device tree, so it'll override
> > "rng-seed" property with whatever device tree entries follow it.
> > However, if rng-seed is the last property (or if the entries that
> > follow are smaller than rng-seed), the seed will stay in memory (or
> > part of it).
> >
> > fdt_nop_property in v2 would erase it for sure. I don't know if there
> > is a way to make sure that rng-seed is removed for good while still
> > deleting the property (maybe modify fdt_splice_ to do a memset(.., 0)
> > of the moved chunk?).
> >
> So maybe we can use fdt_nop_property() back?

Yes I prefer fdt_nop_property, if we don't want to modify delprop or
splice. But it'd be good if others can chime in.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan()
  2019-05-19 16:04 [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan() Hsin-Yi Wang
                   ` (2 preceding siblings ...)
  2019-05-19 23:57 ` [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan() Nicolas Boichat
@ 2019-05-24  0:04 ` Stephen Boyd
  2019-05-25  9:30   ` Mike Rapoport
  2019-05-24 21:58 ` Rob Herring
  4 siblings, 1 reply; 10+ messages in thread
From: Stephen Boyd @ 2019-05-24  0:04 UTC (permalink / raw)
  To: Hsin-Yi Wang, linux-arm-kernel
  Cc: Rob Herring, devicetree, linux-kernel, Frank Rowand,
	Catalin Marinas, Will Deacon, Andrew Morton, Mike Rapoport,
	Ard Biesheuvel, Miles Chen, Hsin-Yi Wang, James Morse,
	Andrew Murray, Mark Rutland, Jun Yao, Yu Zhao, Robin Murphy,
	Laura Abbott, Kees Cook

Quoting Hsin-Yi Wang (2019-05-19 09:04:44)
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index a170c6369a68..29648e86f7e5 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -940,12 +940,12 @@ void *__init __fixmap_remap_fdt(phys_addr_t dt_phys, int *size, pgprot_t prot)
>         return dt_virt;
>  }
>  
> -void *__init fixmap_remap_fdt(phys_addr_t dt_phys)
> +void *__init fixmap_remap_fdt(phys_addr_t dt_phys, pgprot_t prot)
>  {
>         void *dt_virt;
>         int size;
>  
> -       dt_virt = __fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL_RO);
> +       dt_virt = __fixmap_remap_fdt(dt_phys, &size, prot);
>         if (!dt_virt)
>                 return NULL;
>  

Sorry, I'm still confused why we want to call memblock_reserve() again.
Why not avoid it?

diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index 67ef25d037ea..d0d9de9da5c1 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -137,7 +137,7 @@ extern void init_mem_pgprot(void);
 extern void create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
 			       unsigned long virt, phys_addr_t size,
 			       pgprot_t prot, bool page_mappings_only);
-extern void *fixmap_remap_fdt(phys_addr_t dt_phys);
+extern void *__fixmap_remap_fdt(phys_addr_t dt_phys, int *size, pgprot_t prot);
 extern void mark_linear_text_alias_ro(void);
 
 #define INIT_MM_CONTEXT(name)	\
diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index b09b6f75f759..0701c2cf1534 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -65,9 +65,6 @@ static __init const u8 *kaslr_get_cmdline(void *fdt)
 	return default_cmdline;
 }
 
-extern void *__init __fixmap_remap_fdt(phys_addr_t dt_phys, int *size,
-				       pgprot_t prot);
-
 /*
  * This routine will be executed with the kernel mapped at its default virtual
  * address, and if it returns successfully, the kernel will be remapped, and
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 413d566405d1..3e97354566ff 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -181,9 +181,13 @@ static void __init smp_build_mpidr_hash(void)
 
 static void __init setup_machine_fdt(phys_addr_t dt_phys)
 {
-	void *dt_virt = fixmap_remap_fdt(dt_phys);
+	int size;
+	void *dt_virt = __fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL);
 	const char *name;
 
+	if (dt_virt)
+		memblock_reserve(dt_phys, size);
+
 	if (!dt_virt || !early_init_dt_scan(dt_virt)) {
 		pr_crit("\n"
 			"Error: invalid device tree blob at physical address %pa (virtual address 0x%p)\n"
@@ -195,6 +199,9 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys)
 			cpu_relax();
 	}
 
+	/* Early fixups are done, map the FDT as read-only now */
+	__fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL_RO);
+
 	name = of_flat_dt_get_machine_name();
 	if (!name)
 		return;
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index a170c6369a68..ddf6086cd9dd 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -940,19 +940,6 @@ void *__init __fixmap_remap_fdt(phys_addr_t dt_phys, int *size, pgprot_t prot)
 	return dt_virt;
 }
 
-void *__init fixmap_remap_fdt(phys_addr_t dt_phys)
-{
-	void *dt_virt;
-	int size;
-
-	dt_virt = __fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL_RO);
-	if (!dt_virt)
-		return NULL;
-
-	memblock_reserve(dt_phys, size);
-	return dt_virt;
-}
-
 int __init arch_ioremap_pud_supported(void)
 {
 	/* only 4k granule supports level 1 block mappings */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan()
  2019-05-19 16:04 [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan() Hsin-Yi Wang
                   ` (3 preceding siblings ...)
  2019-05-24  0:04 ` Stephen Boyd
@ 2019-05-24 21:58 ` Rob Herring
  4 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2019-05-24 21:58 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: linux-arm-kernel, devicetree, linux-kernel, Frank Rowand,
	Catalin Marinas, Will Deacon, Andrew Morton, Mike Rapoport,
	Ard Biesheuvel, Miles Chen, James Morse, Andrew Murray,
	Mark Rutland, Jun Yao, Yu Zhao, Robin Murphy, Laura Abbott,
	Stephen Boyd, Kees Cook

On Mon, May 20, 2019 at 12:04:44AM +0800, Hsin-Yi Wang wrote:
> Currently in arm64, FDT is mapped to RO before it's passed to
> early_init_dt_scan(). However, there might be some code that needs
> to modify FDT during init. Map FDT to RW until unflatten DT.

typo in the subject.

Otherwise, this one seems fine to me.

> 
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> change log v2->v4:
> * v3 abandoned
> * add an arg pgprot_t to fixmap_remap_fdt()
> ---
>  arch/arm64/include/asm/mmu.h | 2 +-
>  arch/arm64/kernel/setup.c    | 5 ++++-
>  arch/arm64/mm/mmu.c          | 4 ++--
>  3 files changed, 7 insertions(+), 4 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan()
  2019-05-24  0:04 ` Stephen Boyd
@ 2019-05-25  9:30   ` Mike Rapoport
  0 siblings, 0 replies; 10+ messages in thread
From: Mike Rapoport @ 2019-05-25  9:30 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Hsin-Yi Wang, linux-arm-kernel, Rob Herring, devicetree,
	linux-kernel, Frank Rowand, Catalin Marinas, Will Deacon,
	Andrew Morton, Ard Biesheuvel, Miles Chen, James Morse,
	Andrew Murray, Mark Rutland, Jun Yao, Yu Zhao, Robin Murphy,
	Laura Abbott, Kees Cook

On Thu, May 23, 2019 at 05:04:18PM -0700, Stephen Boyd wrote:
> Quoting Hsin-Yi Wang (2019-05-19 09:04:44)
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index a170c6369a68..29648e86f7e5 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -940,12 +940,12 @@ void *__init __fixmap_remap_fdt(phys_addr_t dt_phys, int *size, pgprot_t prot)
> >         return dt_virt;
> >  }
> >  
> > -void *__init fixmap_remap_fdt(phys_addr_t dt_phys)
> > +void *__init fixmap_remap_fdt(phys_addr_t dt_phys, pgprot_t prot)
> >  {
> >         void *dt_virt;
> >         int size;
> >  
> > -       dt_virt = __fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL_RO);
> > +       dt_virt = __fixmap_remap_fdt(dt_phys, &size, prot);
> >         if (!dt_virt)
> >                 return NULL;
> >  
> 
> Sorry, I'm still confused why we want to call memblock_reserve() again.
> Why not avoid it?
 
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 67ef25d037ea..d0d9de9da5c1 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -137,7 +137,7 @@ extern void init_mem_pgprot(void);
>  extern void create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
>  			       unsigned long virt, phys_addr_t size,
>  			       pgprot_t prot, bool page_mappings_only);
> -extern void *fixmap_remap_fdt(phys_addr_t dt_phys);
> +extern void *__fixmap_remap_fdt(phys_addr_t dt_phys, int *size, pgprot_t prot);

I'd drop '__' prefix from __fixmap_remap_fdt is it's the only function to
remain.
Otherwise makes perfect sense.

>  extern void mark_linear_text_alias_ro(void);
>  
>  #define INIT_MM_CONTEXT(name)	\
> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> index b09b6f75f759..0701c2cf1534 100644
> --- a/arch/arm64/kernel/kaslr.c
> +++ b/arch/arm64/kernel/kaslr.c
> @@ -65,9 +65,6 @@ static __init const u8 *kaslr_get_cmdline(void *fdt)
>  	return default_cmdline;
>  }
>  
> -extern void *__init __fixmap_remap_fdt(phys_addr_t dt_phys, int *size,
> -				       pgprot_t prot);
> -
>  /*
>   * This routine will be executed with the kernel mapped at its default virtual
>   * address, and if it returns successfully, the kernel will be remapped, and
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 413d566405d1..3e97354566ff 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -181,9 +181,13 @@ static void __init smp_build_mpidr_hash(void)
>  
>  static void __init setup_machine_fdt(phys_addr_t dt_phys)
>  {
> -	void *dt_virt = fixmap_remap_fdt(dt_phys);
> +	int size;
> +	void *dt_virt = __fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL);
>  	const char *name;
>  
> +	if (dt_virt)
> +		memblock_reserve(dt_phys, size);
> +
>  	if (!dt_virt || !early_init_dt_scan(dt_virt)) {
>  		pr_crit("\n"
>  			"Error: invalid device tree blob at physical address %pa (virtual address 0x%p)\n"
> @@ -195,6 +199,9 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys)
>  			cpu_relax();
>  	}
>  
> +	/* Early fixups are done, map the FDT as read-only now */
> +	__fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL_RO);
> +
>  	name = of_flat_dt_get_machine_name();
>  	if (!name)
>  		return;
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index a170c6369a68..ddf6086cd9dd 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -940,19 +940,6 @@ void *__init __fixmap_remap_fdt(phys_addr_t dt_phys, int *size, pgprot_t prot)
>  	return dt_virt;
>  }
>  
> -void *__init fixmap_remap_fdt(phys_addr_t dt_phys)
> -{
> -	void *dt_virt;
> -	int size;
> -
> -	dt_virt = __fixmap_remap_fdt(dt_phys, &size, PAGE_KERNEL_RO);
> -	if (!dt_virt)
> -		return NULL;
> -
> -	memblock_reserve(dt_phys, size);
> -	return dt_virt;
> -}
> -
>  int __init arch_ioremap_pud_supported(void)
>  {
>  	/* only 4k granule supports level 1 block mappings */
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-05-25  9:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-19 16:04 [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan() Hsin-Yi Wang
2019-05-19 16:04 ` [PATCH v4 2/3] fdt: add support for rng-seed Hsin-Yi Wang
2019-05-19 23:54   ` Nicolas Boichat
2019-05-21  4:09     ` Hsin-Yi Wang
2019-05-21 12:42       ` Nicolas Boichat
2019-05-19 16:04 ` [PATCH v4 3/3] arm64: kexec_file: add rng-seed support Hsin-Yi Wang
2019-05-19 23:57 ` [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan() Nicolas Boichat
2019-05-24  0:04 ` Stephen Boyd
2019-05-25  9:30   ` Mike Rapoport
2019-05-24 21:58 ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).