2019-05-19 17:11:36

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan()

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 <[email protected]>
---
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



2019-05-19 18:23:48

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH v4 3/3] arm64: kexec_file: add rng-seed support

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 <[email protected]>
---
* 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


2019-05-20 03:18:41

by Nicolas Boichat

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan()

s/amr64/arm64/ in the commit title.

On Mon, May 20, 2019 at 1:09 AM Hsin-Yi Wang <[email protected]> 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 <[email protected]>
> ---
> 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
>

2019-05-24 00:06:12

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan()

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 */

2019-05-24 22:00:22

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan()

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 <[email protected]>
> ---
> 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(-)

2019-05-25 09:32:30

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] amr64: map FDT as RW for early_init_dt_scan()

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.