diff mbox

[1/8] of/fdt: split off FDT self reservation from memreserve processing

Message ID 1431326520-17331-2-git-send-email-ard.biesheuvel@linaro.org
State Superseded
Headers show

Commit Message

Ard Biesheuvel May 11, 2015, 6:41 a.m. UTC
This splits off the reservation of the memory occupied by the FDT
binary itself from the processing of the memory reservations it
contains. This is necessary because the physical address of the FDT,
which is needed to perform the reservation, may not be known to the
FDT driver core, i.e., it may be mapped outside the linear direct
mapping, in which case __pa() returns a bogus value.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/mm/init.c         |  1 +
 arch/arm64/mm/init.c       |  1 +
 arch/powerpc/kernel/prom.c |  1 +
 drivers/of/fdt.c           | 19 ++++++++++++++-----
 include/linux/of_fdt.h     |  2 ++
 5 files changed, 19 insertions(+), 5 deletions(-)

Comments

Ard Biesheuvel June 1, 2015, 7:56 a.m. UTC | #1
(snip non-LAKML CCs)

On 22 May 2015 at 12:35, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Mon, May 11, 2015 at 08:41:53AM +0200, Ard Biesheuvel wrote:
>> This splits off the reservation of the memory occupied by the FDT
>> binary itself from the processing of the memory reservations it
>> contains. This is necessary because the physical address of the FDT,
>> which is needed to perform the reservation, may not be known to the
>> FDT driver core, i.e., it may be mapped outside the linear direct
>> mapping, in which case __pa() returns a bogus value.
>>
>> Cc: Russell King <linux@arm.linux.org.uk>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Acked-by: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> For the arm64 part:
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks Catalin,

Since there has been virtually no discussion about these patches, I
guess they have missed the window for being considered for inclusion
in v4.2

May I suggest that you at least consider these patches regarding the ID map

http://article.gmane.org/gmane.linux.ports.arm.kernel/411720
http://article.gmane.org/gmane.linux.ports.arm.kernel/411721

since they are self-contained and the first one does fix a potential
problem where the Image placement logic in the stub does not take the
512 MB alignment boundary into account. The second one is a trivial
cleanup.

Perhaps Mark can comment on the desirability to include the FDT
remapping patch (which depends on this 1/8).

http://article.gmane.org/gmane.linux.kernel.efi/5738

I don't have strong preference either way, although it would be good
to get some patches off my personal queue.

Regards,
Ard.
Ard Biesheuvel June 1, 2015, 10:46 a.m. UTC | #2
On 1 June 2015 at 11:56, Mark Rutland <mark.rutland@arm.com> wrote:
> On Mon, Jun 01, 2015 at 08:56:07AM +0100, Ard Biesheuvel wrote:
>> (snip non-LAKML CCs)
>>
>> On 22 May 2015 at 12:35, Catalin Marinas <catalin.marinas@arm.com> wrote:
>> > On Mon, May 11, 2015 at 08:41:53AM +0200, Ard Biesheuvel wrote:
>> >> This splits off the reservation of the memory occupied by the FDT
>> >> binary itself from the processing of the memory reservations it
>> >> contains. This is necessary because the physical address of the FDT,
>> >> which is needed to perform the reservation, may not be known to the
>> >> FDT driver core, i.e., it may be mapped outside the linear direct
>> >> mapping, in which case __pa() returns a bogus value.
>> >>
>> >> Cc: Russell King <linux@arm.linux.org.uk>
>> >> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> >> Cc: Paul Mackerras <paulus@samba.org>
>> >> Acked-by: Rob Herring <robh@kernel.org>
>> >> Acked-by: Mark Rutland <mark.rutland@arm.com>
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >
>> > For the arm64 part:
>> >
>> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>>
>> Thanks Catalin,
>>
>> Since there has been virtually no discussion about these patches, I
>> guess they have missed the window for being considered for inclusion
>> in v4.2
>>
>> May I suggest that you at least consider these patches regarding the ID map
>>
>> http://article.gmane.org/gmane.linux.ports.arm.kernel/411720
>> http://article.gmane.org/gmane.linux.ports.arm.kernel/411721
>>
>> since they are self-contained and the first one does fix a potential
>> problem where the Image placement logic in the stub does not take the
>> 512 MB alignment boundary into account. The second one is a trivial
>> cleanup.
>
> FWIW both of these look good to me.
>
>> Perhaps Mark can comment on the desirability to include the FDT
>> remapping patch (which depends on this 1/8).
>>
>> http://article.gmane.org/gmane.linux.kernel.efi/5738
>
> I would like to see that taken if possible.
>

Thanks.

Actually, it does make kind of sense to take these four (i.e., this
1/8 plus the three referenced above) patches as a set, since together
they address all the known shortcomings in the EFI stub regarding the
placement of both the FDT and the kernel image. All the other stuff
can easily be deferred.

I will respin these 4 with the new tags added.
Ard Biesheuvel June 1, 2015, 11:14 a.m. UTC | #3
On 1 June 2015 at 13:02, Mark Rutland <mark.rutland@arm.com> wrote:
> On Mon, Jun 01, 2015 at 11:46:27AM +0100, Ard Biesheuvel wrote:
>> On 1 June 2015 at 11:56, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Mon, Jun 01, 2015 at 08:56:07AM +0100, Ard Biesheuvel wrote:
>> >> (snip non-LAKML CCs)
>> >>
>> >> On 22 May 2015 at 12:35, Catalin Marinas <catalin.marinas@arm.com> wrote:
>> >> > On Mon, May 11, 2015 at 08:41:53AM +0200, Ard Biesheuvel wrote:
>> >> >> This splits off the reservation of the memory occupied by the FDT
>> >> >> binary itself from the processing of the memory reservations it
>> >> >> contains. This is necessary because the physical address of the FDT,
>> >> >> which is needed to perform the reservation, may not be known to the
>> >> >> FDT driver core, i.e., it may be mapped outside the linear direct
>> >> >> mapping, in which case __pa() returns a bogus value.
>> >> >>
>> >> >> Cc: Russell King <linux@arm.linux.org.uk>
>> >> >> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> >> >> Cc: Paul Mackerras <paulus@samba.org>
>> >> >> Acked-by: Rob Herring <robh@kernel.org>
>> >> >> Acked-by: Mark Rutland <mark.rutland@arm.com>
>> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> >
>> >> > For the arm64 part:
>> >> >
>> >> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>> >>
>> >> Thanks Catalin,
>> >>
>> >> Since there has been virtually no discussion about these patches, I
>> >> guess they have missed the window for being considered for inclusion
>> >> in v4.2
>> >>
>> >> May I suggest that you at least consider these patches regarding the ID map
>> >>
>> >> http://article.gmane.org/gmane.linux.ports.arm.kernel/411720
>> >> http://article.gmane.org/gmane.linux.ports.arm.kernel/411721
>> >>
>> >> since they are self-contained and the first one does fix a potential
>> >> problem where the Image placement logic in the stub does not take the
>> >> 512 MB alignment boundary into account. The second one is a trivial
>> >> cleanup.
>> >
>> > FWIW both of these look good to me.
>> >
>> >> Perhaps Mark can comment on the desirability to include the FDT
>> >> remapping patch (which depends on this 1/8).
>> >>
>> >> http://article.gmane.org/gmane.linux.kernel.efi/5738
>> >
>> > I would like to see that taken if possible.
>> >
>>
>> Thanks.
>>
>> Actually, it does make kind of sense to take these four (i.e., this
>> 1/8 plus the three referenced above) patches as a set, since together
>> they address all the known shortcomings in the EFI stub regarding the
>> placement of both the FDT and the kernel image. All the other stuff
>> can easily be deferred.
>
> Putting those together as a cleanup+preparation series makes sense to
> me, if that makes it easy for Catalin to pick them up now.
>
> Do we have/need acks from Ben or Paul on this reservation patch?
>

Considering that this patch is essentially a partial revert of
d1552ce449eb0 ("of/fdt: move memreserve and dtb memory reservations
into core") by Rob, who acked this patch and submitted his patch
without the acks of either Ben or Paul, and the fact that Ben and Paul
(and Russell) have all been cc'ed at least 3 times on this patch, my
take would be that it should be fine to go ahead without them. But it
is ultimately up to Catalin, of course.
diff mbox

Patch

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index be92fa0f2f35..8a63b4cdc0f2 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -268,6 +268,7 @@  void __init arm_memblock_init(const struct machine_desc *mdesc)
 	if (mdesc->reserve)
 		mdesc->reserve();
 
+	early_init_fdt_reserve_self();
 	early_init_fdt_scan_reserved_mem();
 
 	/* reserve memory for DMA contiguous allocations */
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 597831bdddf3..89a05f467ffb 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -170,6 +170,7 @@  void __init arm64_memblock_init(void)
 		memblock_reserve(__virt_to_phys(initrd_start), initrd_end - initrd_start);
 #endif
 
+	early_init_fdt_reserve_self();
 	early_init_fdt_scan_reserved_mem();
 
 	/* 4GB maximum for 32-bit only capable devices */
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 308c5e15676b..51ea36f79307 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -573,6 +573,7 @@  static void __init early_reserve_mem_dt(void)
 	int len;
 	const __be32 *prop;
 
+	early_init_fdt_reserve_self();
 	early_init_fdt_scan_reserved_mem();
 
 	dt_root = of_get_flat_dt_root();
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index cde35c5d0191..f2dd23a32267 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -580,11 +580,6 @@  void __init early_init_fdt_scan_reserved_mem(void)
 	if (!initial_boot_params)
 		return;
 
-	/* Reserve the dtb region */
-	early_init_dt_reserve_memory_arch(__pa(initial_boot_params),
-					  fdt_totalsize(initial_boot_params),
-					  0);
-
 	/* Process header /memreserve/ fields */
 	for (n = 0; ; n++) {
 		fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
@@ -598,6 +593,20 @@  void __init early_init_fdt_scan_reserved_mem(void)
 }
 
 /**
+ * early_init_fdt_reserve_self() - reserve the memory used by the FDT blob
+ */
+void __init early_init_fdt_reserve_self(void)
+{
+	if (!initial_boot_params)
+		return;
+
+	/* Reserve the dtb region */
+	early_init_dt_reserve_memory_arch(__pa(initial_boot_params),
+					  fdt_totalsize(initial_boot_params),
+					  0);
+}
+
+/**
  * of_scan_flat_dt - scan flattened tree blob and call callback on each.
  * @it: callback function
  * @data: context data pointer
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 587ee507965d..fd627a58068f 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -64,6 +64,7 @@  extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
 extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
 				     int depth, void *data);
 extern void early_init_fdt_scan_reserved_mem(void);
+extern void early_init_fdt_reserve_self(void);
 extern void early_init_dt_add_memory_arch(u64 base, u64 size);
 extern int early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
 					     bool no_map);
@@ -91,6 +92,7 @@  extern u64 fdt_translate_address(const void *blob, int node_offset);
 extern void of_fdt_limit_memory(int limit);
 #else /* CONFIG_OF_FLATTREE */
 static inline void early_init_fdt_scan_reserved_mem(void) {}
+static inline void early_init_fdt_reserve_self(void) {}
 static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
 static inline void unflatten_device_tree(void) {}
 static inline void unflatten_and_copy_device_tree(void) {}