diff mbox

[Linaro-mm-sig,v2,4/5] ARM: init: add support for reserved memory defined by device tree

Message ID 1391515773-6112-5-git-send-email-m.szyprowski@samsung.com
State New
Headers show

Commit Message

Marek Szyprowski Feb. 4, 2014, 12:09 p.m. UTC
Enable reserved memory initialization from device tree.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Laura Abbott <lauraa@codeaurora.org>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mm/init.c |    3 +++
 1 file changed, 3 insertions(+)

Comments

Grant Likely Feb. 5, 2014, 10:15 a.m. UTC | #1
On Tue, 04 Feb 2014 13:09:32 +0100, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Enable reserved memory initialization from device tree.
> 
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Laura Abbott <lauraa@codeaurora.org>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/mm/init.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> index 804d61566a53..ebafdb479410 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> @@ -17,6 +17,7 @@
>  #include <linux/nodemask.h>
>  #include <linux/initrd.h>
>  #include <linux/of_fdt.h>
> +#include <linux/of_reserved_mem.h>
>  #include <linux/highmem.h>
>  #include <linux/gfp.h>
>  #include <linux/memblock.h>
> @@ -323,6 +324,8 @@ void __init arm_memblock_init(struct meminfo *mi,
>  	if (mdesc->reserve)
>  		mdesc->reserve();
>  
> +	early_init_dt_scan_reserved_mem();
> +

The new binding is being made fundamental. If the reserved-memory node
is present, then it needs to be honored, even if the kernel doesn't know
how to use the regions. Therefore, This needs to be unconditional for
all architectures. The hook should be called in early_init_dt_scan()
(drivers/of/fdt.c) immediately after the early_init_dt_scan_memory()
hook.

>  	/*
>  	 * reserve memory for DMA contigouos allocations,
>  	 * must come from DMA area inside low memory
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Marek Szyprowski Feb. 6, 2014, 1:26 p.m. UTC | #2
Hello,

On 2014-02-05 11:15, Grant Likely wrote:
> On Tue, 04 Feb 2014 13:09:32 +0100, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > Enable reserved memory initialization from device tree.
> >
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Laura Abbott <lauraa@codeaurora.org>
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > ---
> >  arch/arm/mm/init.c |    3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> > index 804d61566a53..ebafdb479410 100644
> > --- a/arch/arm/mm/init.c
> > +++ b/arch/arm/mm/init.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/nodemask.h>
> >  #include <linux/initrd.h>
> >  #include <linux/of_fdt.h>
> > +#include <linux/of_reserved_mem.h>
> >  #include <linux/highmem.h>
> >  #include <linux/gfp.h>
> >  #include <linux/memblock.h>
> > @@ -323,6 +324,8 @@ void __init arm_memblock_init(struct meminfo *mi,
> >  	if (mdesc->reserve)
> >  		mdesc->reserve();
> >
> > +	early_init_dt_scan_reserved_mem();
> > +
>
> The new binding is being made fundamental. If the reserved-memory node
> is present, then it needs to be honored, even if the kernel doesn't know
> how to use the regions. Therefore, This needs to be unconditional for
> all architectures. The hook should be called in early_init_dt_scan()
> (drivers/of/fdt.c) immediately after the early_init_dt_scan_memory()
> hook.

In theory this will be the best solution, but it practice there is a
problem. early_init_dt_scan() is called as the first function from kernel
booting code. That time there is no memory yet added to the system, so it
would be really hard to reserve anything. Memory nodes are being added
later either with memblock_add() or by some other arch specific way.

Finally, once all memory has been added to the system we can parse and
reserve all regions defined in the device tree. This really requires
creating another function which will be called by arch specific code.

Best regards
Grant Likely Feb. 10, 2014, 9:59 p.m. UTC | #3
On Thu, 06 Feb 2014 14:26:13 +0100, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Hello,
> 
> On 2014-02-05 11:15, Grant Likely wrote:
> > On Tue, 04 Feb 2014 13:09:32 +0100, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > > Enable reserved memory initialization from device tree.
> > >
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Laura Abbott <lauraa@codeaurora.org>
> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > ---
> > >  arch/arm/mm/init.c |    3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> > > index 804d61566a53..ebafdb479410 100644
> > > --- a/arch/arm/mm/init.c
> > > +++ b/arch/arm/mm/init.c
> > > @@ -17,6 +17,7 @@
> > >  #include <linux/nodemask.h>
> > >  #include <linux/initrd.h>
> > >  #include <linux/of_fdt.h>
> > > +#include <linux/of_reserved_mem.h>
> > >  #include <linux/highmem.h>
> > >  #include <linux/gfp.h>
> > >  #include <linux/memblock.h>
> > > @@ -323,6 +324,8 @@ void __init arm_memblock_init(struct meminfo *mi,
> > >  	if (mdesc->reserve)
> > >  		mdesc->reserve();
> > >
> > > +	early_init_dt_scan_reserved_mem();
> > > +
> >
> > The new binding is being made fundamental. If the reserved-memory node
> > is present, then it needs to be honored, even if the kernel doesn't know
> > how to use the regions. Therefore, This needs to be unconditional for
> > all architectures. The hook should be called in early_init_dt_scan()
> > (drivers/of/fdt.c) immediately after the early_init_dt_scan_memory()
> > hook.
> 
> In theory this will be the best solution, but it practice there is a
> problem. early_init_dt_scan() is called as the first function from kernel
> booting code. That time there is no memory yet added to the system, so it
> would be really hard to reserve anything. Memory nodes are being added
> later either with memblock_add() or by some other arch specific way.

Hmmm, depends on the architecture. On ARM the memory is loaded into the
meminfo structure first, and it isn't until arm_memblock_init() that
memblock_add() gets called on all the regions. Some architectures do the
memblock_add() directly from early_init_dt_add_memory_arch() function.

The default early_init_dt_add_memory_arch() in drivers/of/fdt.c is
overridden by ARM and a number of other architectures. However...

> Finally, once all memory has been added to the system we can parse and
> reserve all regions defined in the device tree. This really requires
> creating another function which will be called by arch specific code.

...Or it means getting rid of meminfo entirely so that memblock is
available earlier. Laura Abbott has just posted v2 of her series to do
exactly that. If you base on that then you should be able to do exactly
what I suggested.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Marek Szyprowski Feb. 11, 2014, 10:52 a.m. UTC | #4
On 2014-02-10 22:59, Grant Likely wrote:
> On Thu, 06 Feb 2014 14:26:13 +0100, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > Hello,
> >
> > On 2014-02-05 11:15, Grant Likely wrote:
> > > On Tue, 04 Feb 2014 13:09:32 +0100, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > > > Enable reserved memory initialization from device tree.
> > > >
> > > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > > Cc: Laura Abbott <lauraa@codeaurora.org>
> > > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > > ---
> > > >  arch/arm/mm/init.c |    3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> > > > index 804d61566a53..ebafdb479410 100644
> > > > --- a/arch/arm/mm/init.c
> > > > +++ b/arch/arm/mm/init.c
> > > > @@ -17,6 +17,7 @@
> > > >  #include <linux/nodemask.h>
> > > >  #include <linux/initrd.h>
> > > >  #include <linux/of_fdt.h>
> > > > +#include <linux/of_reserved_mem.h>
> > > >  #include <linux/highmem.h>
> > > >  #include <linux/gfp.h>
> > > >  #include <linux/memblock.h>
> > > > @@ -323,6 +324,8 @@ void __init arm_memblock_init(struct meminfo *mi,
> > > >  	if (mdesc->reserve)
> > > >  		mdesc->reserve();
> > > >
> > > > +	early_init_dt_scan_reserved_mem();
> > > > +
> > >
> > > The new binding is being made fundamental. If the reserved-memory node
> > > is present, then it needs to be honored, even if the kernel doesn't know
> > > how to use the regions. Therefore, This needs to be unconditional for
> > > all architectures. The hook should be called in early_init_dt_scan()
> > > (drivers/of/fdt.c) immediately after the early_init_dt_scan_memory()
> > > hook.
> >
> > In theory this will be the best solution, but it practice there is a
> > problem. early_init_dt_scan() is called as the first function from kernel
> > booting code. That time there is no memory yet added to the system, so it
> > would be really hard to reserve anything. Memory nodes are being added
> > later either with memblock_add() or by some other arch specific way.
>
> Hmmm, depends on the architecture. On ARM the memory is loaded into the
> meminfo structure first, and it isn't until arm_memblock_init() that
> memblock_add() gets called on all the regions. Some architectures do the
> memblock_add() directly from early_init_dt_add_memory_arch() function.
>
> The default early_init_dt_add_memory_arch() in drivers/of/fdt.c is
> overridden by ARM and a number of other architectures. However...
>
> > Finally, once all memory has been added to the system we can parse and
> > reserve all regions defined in the device tree. This really requires
> > creating another function which will be called by arch specific code.
>
> ...Or it means getting rid of meminfo entirely so that memblock is
> available earlier. Laura Abbott has just posted v2 of her series to do
> exactly that. If you base on that then you should be able to do exactly
> what I suggested.

I've checked Laura's patches and in fact it is possible to do memory
reservation as a last step in early_init_dt_scan_memory(). However still
see some problem which I have no idea how to resolve. Right now I focus
only on ARM, so I have no idea how it is solved by other architectures.
On of the key features of the new binding is the ability to automatically
allocate reserved regions of the given size. However kernel, initrd, dt
and other sub-arch specific critical regions are marked/allocated in
arm_memblock_init(), which is called after setup_machine_fdt(). This
might lead to some serious failures when automatically reserved region
overlaps with some critical resources. Do you have any idea how to solve
this without a new callback?

Best regards
Grant Likely Feb. 11, 2014, 11:50 a.m. UTC | #5
On Tue, 11 Feb 2014 11:52:42 +0100, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> 
> On 2014-02-10 22:59, Grant Likely wrote:
> > On Thu, 06 Feb 2014 14:26:13 +0100, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > > Hello,
> > >
> > > On 2014-02-05 11:15, Grant Likely wrote:
> > > > On Tue, 04 Feb 2014 13:09:32 +0100, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > > > > Enable reserved memory initialization from device tree.
> > > > >
> > > > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > > > Cc: Laura Abbott <lauraa@codeaurora.org>
> > > > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > > > ---
> > > > >  arch/arm/mm/init.c |    3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> > > > > index 804d61566a53..ebafdb479410 100644
> > > > > --- a/arch/arm/mm/init.c
> > > > > +++ b/arch/arm/mm/init.c
> > > > > @@ -17,6 +17,7 @@
> > > > >  #include <linux/nodemask.h>
> > > > >  #include <linux/initrd.h>
> > > > >  #include <linux/of_fdt.h>
> > > > > +#include <linux/of_reserved_mem.h>
> > > > >  #include <linux/highmem.h>
> > > > >  #include <linux/gfp.h>
> > > > >  #include <linux/memblock.h>
> > > > > @@ -323,6 +324,8 @@ void __init arm_memblock_init(struct meminfo *mi,
> > > > >  	if (mdesc->reserve)
> > > > >  		mdesc->reserve();
> > > > >
> > > > > +	early_init_dt_scan_reserved_mem();
> > > > > +
> > > >
> > > > The new binding is being made fundamental. If the reserved-memory node
> > > > is present, then it needs to be honored, even if the kernel doesn't know
> > > > how to use the regions. Therefore, This needs to be unconditional for
> > > > all architectures. The hook should be called in early_init_dt_scan()
> > > > (drivers/of/fdt.c) immediately after the early_init_dt_scan_memory()
> > > > hook.
> > >
> > > In theory this will be the best solution, but it practice there is a
> > > problem. early_init_dt_scan() is called as the first function from kernel
> > > booting code. That time there is no memory yet added to the system, so it
> > > would be really hard to reserve anything. Memory nodes are being added
> > > later either with memblock_add() or by some other arch specific way.
> >
> > Hmmm, depends on the architecture. On ARM the memory is loaded into the
> > meminfo structure first, and it isn't until arm_memblock_init() that
> > memblock_add() gets called on all the regions. Some architectures do the
> > memblock_add() directly from early_init_dt_add_memory_arch() function.
> >
> > The default early_init_dt_add_memory_arch() in drivers/of/fdt.c is
> > overridden by ARM and a number of other architectures. However...
> >
> > > Finally, once all memory has been added to the system we can parse and
> > > reserve all regions defined in the device tree. This really requires
> > > creating another function which will be called by arch specific code.
> >
> > ...Or it means getting rid of meminfo entirely so that memblock is
> > available earlier. Laura Abbott has just posted v2 of her series to do
> > exactly that. If you base on that then you should be able to do exactly
> > what I suggested.
> 
> I've checked Laura's patches and in fact it is possible to do memory
> reservation as a last step in early_init_dt_scan_memory(). However still
> see some problem which I have no idea how to resolve. Right now I focus
> only on ARM, so I have no idea how it is solved by other architectures.

It isn't at the moment because no other architectures are doing dynamic
allocation of regions in this way.

> On of the key features of the new binding is the ability to automatically
> allocate reserved regions of the given size. However kernel, initrd, dt
> and other sub-arch specific critical regions are marked/allocated in
> arm_memblock_init(), which is called after setup_machine_fdt(). This
> might lead to some serious failures when automatically reserved region
> overlaps with some critical resources. Do you have any idea how to solve
> this without a new callback?

Not without moving the kernel/initrd/fdt reservation earlier, which can
certainly be done after Laura's series is applied. It would also be
possible to split up the fixed regions from the dynamic regions so that
the dynamic stuff would be done later.

However, I don't want to block this series on Laura's rework. Go ahead
and keep it as a separate hook, but make the other changes I suggested,
particularly with regard to making the hook architecture independent.
Further rework can be done later after Laura's series is merged.

g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 804d61566a53..ebafdb479410 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -17,6 +17,7 @@ 
 #include <linux/nodemask.h>
 #include <linux/initrd.h>
 #include <linux/of_fdt.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/highmem.h>
 #include <linux/gfp.h>
 #include <linux/memblock.h>
@@ -323,6 +324,8 @@  void __init arm_memblock_init(struct meminfo *mi,
 	if (mdesc->reserve)
 		mdesc->reserve();
 
+	early_init_dt_scan_reserved_mem();
+
 	/*
 	 * reserve memory for DMA contigouos allocations,
 	 * must come from DMA area inside low memory