All of lore.kernel.org
 help / color / mirror / Atom feed
* The alignment mismatch issues between the of_reserved_mem and the CMA setup requirement
@ 2015-11-09  9:39 ` Liu Jason
  0 siblings, 0 replies; 9+ messages in thread
From: Liu Jason @ 2015-11-09  9:39 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-arm-kernel
  Cc: m.szyprowski, grant.likely, robh+dt

There is an alignment mismatch issue between the of_reserved_mem and the CMA setup requirement.

The alignment in Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt


alignment (optional) - length based on parent's #size-cells
                        - Address boundary for alignment of allocation.


But this is not exactly match the CMA setup requirement if the alignment not set or set it not correctly.

The of_reserved_mem will get the alignment from the DTS and pass it to __memblock_alloc_base to
do the memory block allocation. If no alignment property in the DTS, the align will be SMP_CACHE_BYTES

BUT, The CMA setup require the alignment as the following in the code:

align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order)

static int __init rmem_cma_setup(struct reserved_mem *rmem)
{
        phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
        phys_addr_t mask = align - 1;
        unsigned long node = rmem->fdt_node;
        struct cma *cma;
        int err;

        if (!of_get_flat_dt_prop(node, "reusable", NULL) ||
            of_get_flat_dt_prop(node, "no-map", NULL))
                return -EINVAL;

        if ((rmem->base & mask) || (rmem->size & mask)) {
                pr_err("Reserved memory: incorrect alignment of CMA region\n");
                return -EINVAL;
        }
        <snip>
}

So, there is very likely that the alignment mismatch between the of_reserved_mem and the CMA setup requirement.
The sanity check in the rmem_cma_setup will fail and CMA not get set up in the end, this is not expected for CMA.

In the test, there will be following err log when this mismatch happen.

Reserved memory: incorrect alignment of CMA region.

The following patch to fix this issue, any comments? 

=======================================================================================

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 62f467b..a20d4d3 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -124,6 +124,15 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
                align = dt_mem_next_cell(dt_root_addr_cells, &prop);
        }

+       if (of_flat_dt_is_compatible(node,"shared-dma-pool")) {
+               phys_addr_t align_required = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
+                if (!align || align != ALIGN(align, align_required)) {
+                        pr_warn("Reserved memory: the alignment not set up correctly in '%s' node."
+                               "change from %pa to %pa \n", uname, &align, &align_required);
+                        align = align_required;
+                }
+        }
+

Best Regards,
Jason Liu


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* The alignment mismatch issues between the of_reserved_mem and the CMA setup requirement
@ 2015-11-09  9:39 ` Liu Jason
  0 siblings, 0 replies; 9+ messages in thread
From: Liu Jason @ 2015-11-09  9:39 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-arm-kernel
  Cc: m.szyprowski, grant.likely, robh+dt

There is an alignment mismatch issue between the of_reserved_mem and the CMA setup requirement.

The alignment in Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt


alignment (optional) - length based on parent's #size-cells
                        - Address boundary for alignment of allocation.


But this is not exactly match the CMA setup requirement if the alignment not set or set it not correctly.

The of_reserved_mem will get the alignment from the DTS and pass it to __memblock_alloc_base to
do the memory block allocation. If no alignment property in the DTS, the align will be SMP_CACHE_BYTES

BUT, The CMA setup require the alignment as the following in the code:

align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order)

static int __init rmem_cma_setup(struct reserved_mem *rmem)
{
        phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
        phys_addr_t mask = align - 1;
        unsigned long node = rmem->fdt_node;
        struct cma *cma;
        int err;

        if (!of_get_flat_dt_prop(node, "reusable", NULL) ||
            of_get_flat_dt_prop(node, "no-map", NULL))
                return -EINVAL;

        if ((rmem->base & mask) || (rmem->size & mask)) {
                pr_err("Reserved memory: incorrect alignment of CMA region\n");
                return -EINVAL;
        }
        <snip>
}

So, there is very likely that the alignment mismatch between the of_reserved_mem and the CMA setup requirement.
The sanity check in the rmem_cma_setup will fail and CMA not get set up in the end, this is not expected for CMA.

In the test, there will be following err log when this mismatch happen.

Reserved memory: incorrect alignment of CMA region.

The following patch to fix this issue, any comments? 

=======================================================================================

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 62f467b..a20d4d3 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -124,6 +124,15 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
                align = dt_mem_next_cell(dt_root_addr_cells, &prop);
        }

+       if (of_flat_dt_is_compatible(node,"shared-dma-pool")) {
+               phys_addr_t align_required = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
+                if (!align || align != ALIGN(align, align_required)) {
+                        pr_warn("Reserved memory: the alignment not set up correctly in '%s' node."
+                               "change from %pa to %pa \n", uname, &align, &align_required);
+                        align = align_required;
+                }
+        }
+

Best Regards,
Jason Liu

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* The alignment mismatch issues between the of_reserved_mem and the CMA setup requirement
@ 2015-11-09  9:39 ` Liu Jason
  0 siblings, 0 replies; 9+ messages in thread
From: Liu Jason @ 2015-11-09  9:39 UTC (permalink / raw)
  To: linux-arm-kernel

There is an alignment mismatch issue between the of_reserved_mem and the CMA setup requirement.

The alignment in Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt


alignment (optional) - length based on parent's #size-cells
? ? ? ? ? ? ? ? ? ? ? ? - Address boundary for alignment of allocation.


But this is not exactly match the CMA setup requirement if the alignment not set or set it not correctly.

The of_reserved_mem will get the alignment from the DTS and pass it to __memblock_alloc_base to
do the memory block allocation. If no alignment property in the DTS, the align will be SMP_CACHE_BYTES

BUT, The CMA setup require the alignment as the following in the code:

align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order)

static int __init rmem_cma_setup(struct reserved_mem *rmem)
{
? ? ? ? phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
? ? ? ? phys_addr_t mask = align - 1;
? ? ? ? unsigned long node = rmem->fdt_node;
? ? ? ? struct cma *cma;
? ? ? ? int err;

? ? ? ? if (!of_get_flat_dt_prop(node, "reusable", NULL) ||
? ? ? ? ? ? of_get_flat_dt_prop(node, "no-map", NULL))
? ? ? ? ? ? ? ? return -EINVAL;

? ? ? ? if ((rmem->base & mask) || (rmem->size & mask)) {
? ? ? ? ? ? ? ? pr_err("Reserved memory: incorrect alignment of CMA region\n");
? ? ? ? ? ? ? ? return -EINVAL;
? ? ? ? }
? ? ? ? <snip>
}

So, there is very likely that the alignment mismatch between the of_reserved_mem and the CMA setup requirement.
The sanity check in the rmem_cma_setup will fail and CMA not get set up in the end, this is not expected for CMA.

In the test, there will be following err log when this mismatch happen.

Reserved memory: incorrect alignment of CMA region.

The following patch to fix this issue, any comments? 

=======================================================================================

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 62f467b..a20d4d3 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -124,6 +124,15 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
                align = dt_mem_next_cell(dt_root_addr_cells, &prop);
        }

+       if (of_flat_dt_is_compatible(node,"shared-dma-pool")) {
+               phys_addr_t align_required = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
+                if (!align || align != ALIGN(align, align_required)) {
+                        pr_warn("Reserved memory: the alignment not set up correctly in '%s' node."
+                               "change from %pa to %pa \n", uname, &align, &align_required);
+                        align = align_required;
+                }
+        }
+

Best Regards,
Jason Liu

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: The alignment mismatch issues between the of_reserved_mem and the CMA setup requirement
@ 2015-11-09 14:25   ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2015-11-09 14:25 UTC (permalink / raw)
  To: Liu Jason
  Cc: devicetree, linux-kernel, linux-arm-kernel, m.szyprowski, grant.likely

On Mon, Nov 9, 2015 at 3:39 AM, Liu Jason <Hui.Liu@freescale.com> wrote:
> There is an alignment mismatch issue between the of_reserved_mem and the CMA setup requirement.
>
> The alignment in Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>
>
> alignment (optional) - length based on parent's #size-cells
>                         - Address boundary for alignment of allocation.
>
>
> But this is not exactly match the CMA setup requirement if the alignment not set or set it not correctly.
>
> The of_reserved_mem will get the alignment from the DTS and pass it to __memblock_alloc_base to
> do the memory block allocation. If no alignment property in the DTS, the align will be SMP_CACHE_BYTES

IMO, any alignment requirement in the DTB should reflect h/w alignment
requirements. We can't know what alignment the OS wants. So if the OS
needs to further increase the alignment, it should adjust the
alignment.

>
> BUT, The CMA setup require the alignment as the following in the code:
>
> align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order)
>
> static int __init rmem_cma_setup(struct reserved_mem *rmem)
> {
>         phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
>         phys_addr_t mask = align - 1;
>         unsigned long node = rmem->fdt_node;
>         struct cma *cma;
>         int err;
>
>         if (!of_get_flat_dt_prop(node, "reusable", NULL) ||
>             of_get_flat_dt_prop(node, "no-map", NULL))
>                 return -EINVAL;
>
>         if ((rmem->base & mask) || (rmem->size & mask)) {
>                 pr_err("Reserved memory: incorrect alignment of CMA region\n");
>                 return -EINVAL;
>         }
>         <snip>
> }
>
> So, there is very likely that the alignment mismatch between the of_reserved_mem and the CMA setup requirement.
> The sanity check in the rmem_cma_setup will fail and CMA not get set up in the end, this is not expected for CMA.
>
> In the test, there will be following err log when this mismatch happen.
>
> Reserved memory: incorrect alignment of CMA region.
>
> The following patch to fix this issue, any comments?
>
> =======================================================================================
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 62f467b..a20d4d3 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -124,6 +124,15 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
>                 align = dt_mem_next_cell(dt_root_addr_cells, &prop);
>         }
>
> +       if (of_flat_dt_is_compatible(node,"shared-dma-pool")) {
> +               phys_addr_t align_required = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
> +                if (!align || align != ALIGN(align, align_required)) {
> +                        pr_warn("Reserved memory: the alignment not set up correctly in '%s' node."
> +                               "change from %pa to %pa \n", uname, &align, &align_required);
> +                        align = align_required;
> +                }
> +        }

You simply need the max required from the DT (which could be more) or
CMA, so this can be simplified to:

align = max(align, PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order));

I don't think you should warn here either.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: The alignment mismatch issues between the of_reserved_mem and the CMA setup requirement
@ 2015-11-09 14:25   ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2015-11-09 14:25 UTC (permalink / raw)
  To: Liu Jason
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A

On Mon, Nov 9, 2015 at 3:39 AM, Liu Jason <Hui.Liu-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
> There is an alignment mismatch issue between the of_reserved_mem and the CMA setup requirement.
>
> The alignment in Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>
>
> alignment (optional) - length based on parent's #size-cells
>                         - Address boundary for alignment of allocation.
>
>
> But this is not exactly match the CMA setup requirement if the alignment not set or set it not correctly.
>
> The of_reserved_mem will get the alignment from the DTS and pass it to __memblock_alloc_base to
> do the memory block allocation. If no alignment property in the DTS, the align will be SMP_CACHE_BYTES

IMO, any alignment requirement in the DTB should reflect h/w alignment
requirements. We can't know what alignment the OS wants. So if the OS
needs to further increase the alignment, it should adjust the
alignment.

>
> BUT, The CMA setup require the alignment as the following in the code:
>
> align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order)
>
> static int __init rmem_cma_setup(struct reserved_mem *rmem)
> {
>         phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
>         phys_addr_t mask = align - 1;
>         unsigned long node = rmem->fdt_node;
>         struct cma *cma;
>         int err;
>
>         if (!of_get_flat_dt_prop(node, "reusable", NULL) ||
>             of_get_flat_dt_prop(node, "no-map", NULL))
>                 return -EINVAL;
>
>         if ((rmem->base & mask) || (rmem->size & mask)) {
>                 pr_err("Reserved memory: incorrect alignment of CMA region\n");
>                 return -EINVAL;
>         }
>         <snip>
> }
>
> So, there is very likely that the alignment mismatch between the of_reserved_mem and the CMA setup requirement.
> The sanity check in the rmem_cma_setup will fail and CMA not get set up in the end, this is not expected for CMA.
>
> In the test, there will be following err log when this mismatch happen.
>
> Reserved memory: incorrect alignment of CMA region.
>
> The following patch to fix this issue, any comments?
>
> =======================================================================================
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 62f467b..a20d4d3 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -124,6 +124,15 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
>                 align = dt_mem_next_cell(dt_root_addr_cells, &prop);
>         }
>
> +       if (of_flat_dt_is_compatible(node,"shared-dma-pool")) {
> +               phys_addr_t align_required = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
> +                if (!align || align != ALIGN(align, align_required)) {
> +                        pr_warn("Reserved memory: the alignment not set up correctly in '%s' node."
> +                               "change from %pa to %pa \n", uname, &align, &align_required);
> +                        align = align_required;
> +                }
> +        }

You simply need the max required from the DT (which could be more) or
CMA, so this can be simplified to:

align = max(align, PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order));

I don't think you should warn here either.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* The alignment mismatch issues between the of_reserved_mem and the CMA setup requirement
@ 2015-11-09 14:25   ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2015-11-09 14:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 9, 2015 at 3:39 AM, Liu Jason <Hui.Liu@freescale.com> wrote:
> There is an alignment mismatch issue between the of_reserved_mem and the CMA setup requirement.
>
> The alignment in Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>
>
> alignment (optional) - length based on parent's #size-cells
>                         - Address boundary for alignment of allocation.
>
>
> But this is not exactly match the CMA setup requirement if the alignment not set or set it not correctly.
>
> The of_reserved_mem will get the alignment from the DTS and pass it to __memblock_alloc_base to
> do the memory block allocation. If no alignment property in the DTS, the align will be SMP_CACHE_BYTES

IMO, any alignment requirement in the DTB should reflect h/w alignment
requirements. We can't know what alignment the OS wants. So if the OS
needs to further increase the alignment, it should adjust the
alignment.

>
> BUT, The CMA setup require the alignment as the following in the code:
>
> align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order)
>
> static int __init rmem_cma_setup(struct reserved_mem *rmem)
> {
>         phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
>         phys_addr_t mask = align - 1;
>         unsigned long node = rmem->fdt_node;
>         struct cma *cma;
>         int err;
>
>         if (!of_get_flat_dt_prop(node, "reusable", NULL) ||
>             of_get_flat_dt_prop(node, "no-map", NULL))
>                 return -EINVAL;
>
>         if ((rmem->base & mask) || (rmem->size & mask)) {
>                 pr_err("Reserved memory: incorrect alignment of CMA region\n");
>                 return -EINVAL;
>         }
>         <snip>
> }
>
> So, there is very likely that the alignment mismatch between the of_reserved_mem and the CMA setup requirement.
> The sanity check in the rmem_cma_setup will fail and CMA not get set up in the end, this is not expected for CMA.
>
> In the test, there will be following err log when this mismatch happen.
>
> Reserved memory: incorrect alignment of CMA region.
>
> The following patch to fix this issue, any comments?
>
> =======================================================================================
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 62f467b..a20d4d3 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -124,6 +124,15 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
>                 align = dt_mem_next_cell(dt_root_addr_cells, &prop);
>         }
>
> +       if (of_flat_dt_is_compatible(node,"shared-dma-pool")) {
> +               phys_addr_t align_required = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
> +                if (!align || align != ALIGN(align, align_required)) {
> +                        pr_warn("Reserved memory: the alignment not set up correctly in '%s' node."
> +                               "change from %pa to %pa \n", uname, &align, &align_required);
> +                        align = align_required;
> +                }
> +        }

You simply need the max required from the DT (which could be more) or
CMA, so this can be simplified to:

align = max(align, PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order));

I don't think you should warn here either.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread
* RE: The alignment mismatch issues between the of_reserved_mem and the CMA setup requirement
  2015-11-09 14:25   ` Rob Herring
  (?)
@ 2015-11-10  3:08     ` Liu Jason
  -1 siblings, 0 replies; 9+ messages in thread
From: Liu Jason @ 2015-11-10  3:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-kernel, linux-arm-kernel, m.szyprowski, grant.likely

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4309 bytes --]

> -----Original Message-----
> From: Rob Herring [mailto:robh+dt@kernel.org]
> Sent: Monday, November 09, 2015 10:26 PM
> To: Liu Hui-R64343
> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; m.szyprowski@samsung.com;
> grant.likely@linaro.org
> Subject: Re: The alignment mismatch issues between the of_reserved_mem
> and the CMA setup requirement
> 
> On Mon, Nov 9, 2015 at 3:39 AM, Liu Jason <Hui.Liu@freescale.com> wrote:
> > There is an alignment mismatch issue between the of_reserved_mem and
> the CMA setup requirement.
> >
> > The alignment in
> > Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> >
> >
> > alignment (optional) - length based on parent's #size-cells
> >                         - Address boundary for alignment of allocation.
> >
> >
> > But this is not exactly match the CMA setup requirement if the
> alignment not set or set it not correctly.
> >
> > The of_reserved_mem will get the alignment from the DTS and pass it to
> > __memblock_alloc_base to do the memory block allocation. If no
> > alignment property in the DTS, the align will be SMP_CACHE_BYTES
> 
> IMO, any alignment requirement in the DTB should reflect h/w alignment
> requirements. We can't know what alignment the OS wants. So if the OS
> needs to further increase the alignment, it should adjust the alignment.
> 

Agree.

> >
> > BUT, The CMA setup require the alignment as the following in the code:
> >
> > align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order)
> >
> > static int __init rmem_cma_setup(struct reserved_mem *rmem) {
> >         phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1,
> pageblock_order);
> >         phys_addr_t mask = align - 1;
> >         unsigned long node = rmem->fdt_node;
> >         struct cma *cma;
> >         int err;
> >
> >         if (!of_get_flat_dt_prop(node, "reusable", NULL) ||
> >             of_get_flat_dt_prop(node, "no-map", NULL))
> >                 return -EINVAL;
> >
> >         if ((rmem->base & mask) || (rmem->size & mask)) {
> >                 pr_err("Reserved memory: incorrect alignment of CMA
> region\n");
> >                 return -EINVAL;
> >         }
> >         <snip>
> > }
> >
> > So, there is very likely that the alignment mismatch between the
> of_reserved_mem and the CMA setup requirement.
> > The sanity check in the rmem_cma_setup will fail and CMA not get set up
> in the end, this is not expected for CMA.
> >
> > In the test, there will be following err log when this mismatch happen.
> >
> > Reserved memory: incorrect alignment of CMA region.
> >
> > The following patch to fix this issue, any comments?
> >
> > ======================================================================
> > =================
> >
> > diff --git a/drivers/of/of_reserved_mem.c
> > b/drivers/of/of_reserved_mem.c index 62f467b..a20d4d3 100644
> > --- a/drivers/of/of_reserved_mem.c
> > +++ b/drivers/of/of_reserved_mem.c
> > @@ -124,6 +124,15 @@ static int __init
> __reserved_mem_alloc_size(unsigned long node,
> >                 align = dt_mem_next_cell(dt_root_addr_cells, &prop);
> >         }
> >
> > +       if (of_flat_dt_is_compatible(node,"shared-dma-pool")) {
> > +               phys_addr_t align_required = PAGE_SIZE << max(MAX_ORDER
> - 1, pageblock_order);
> > +                if (!align || align != ALIGN(align, align_required)) {
> > +                        pr_warn("Reserved memory: the alignment not
> set up correctly in '%s' node."
> > +                               "change from %pa to %pa \n", uname,
> &align, &align_required);
> > +                        align = align_required;
> > +                }
> > +        }
> 
> You simply need the max required from the DT (which could be more) or CMA,
> so this can be simplified to:
> 
> align = max(align, PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order));
> 
> I don't think you should warn here either.

If don't want to give some message to tell that the alignment in the DTS specified has been
Updated, the code can be simpler as you said. 


I can submit one patch for this if there is no more comments on this. 

> 
> Rob
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 9+ messages in thread
* RE: The alignment mismatch issues between the of_reserved_mem and the CMA setup requirement
@ 2015-11-10  3:08     ` Liu Jason
  0 siblings, 0 replies; 9+ messages in thread
From: Liu Jason @ 2015-11-10  3:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: grant.likely, devicetree, linux-kernel, linux-arm-kernel, m.szyprowski

> -----Original Message-----
> From: Rob Herring [mailto:robh+dt@kernel.org]
> Sent: Monday, November 09, 2015 10:26 PM
> To: Liu Hui-R64343
> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; m.szyprowski@samsung.com;
> grant.likely@linaro.org
> Subject: Re: The alignment mismatch issues between the of_reserved_mem
> and the CMA setup requirement
> 
> On Mon, Nov 9, 2015 at 3:39 AM, Liu Jason <Hui.Liu@freescale.com> wrote:
> > There is an alignment mismatch issue between the of_reserved_mem and
> the CMA setup requirement.
> >
> > The alignment in
> > Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> >
> >
> > alignment (optional) - length based on parent's #size-cells
> >                         - Address boundary for alignment of allocation.
> >
> >
> > But this is not exactly match the CMA setup requirement if the
> alignment not set or set it not correctly.
> >
> > The of_reserved_mem will get the alignment from the DTS and pass it to
> > __memblock_alloc_base to do the memory block allocation. If no
> > alignment property in the DTS, the align will be SMP_CACHE_BYTES
> 
> IMO, any alignment requirement in the DTB should reflect h/w alignment
> requirements. We can't know what alignment the OS wants. So if the OS
> needs to further increase the alignment, it should adjust the alignment.
> 

Agree.

> >
> > BUT, The CMA setup require the alignment as the following in the code:
> >
> > align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order)
> >
> > static int __init rmem_cma_setup(struct reserved_mem *rmem) {
> >         phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1,
> pageblock_order);
> >         phys_addr_t mask = align - 1;
> >         unsigned long node = rmem->fdt_node;
> >         struct cma *cma;
> >         int err;
> >
> >         if (!of_get_flat_dt_prop(node, "reusable", NULL) ||
> >             of_get_flat_dt_prop(node, "no-map", NULL))
> >                 return -EINVAL;
> >
> >         if ((rmem->base & mask) || (rmem->size & mask)) {
> >                 pr_err("Reserved memory: incorrect alignment of CMA
> region\n");
> >                 return -EINVAL;
> >         }
> >         <snip>
> > }
> >
> > So, there is very likely that the alignment mismatch between the
> of_reserved_mem and the CMA setup requirement.
> > The sanity check in the rmem_cma_setup will fail and CMA not get set up
> in the end, this is not expected for CMA.
> >
> > In the test, there will be following err log when this mismatch happen.
> >
> > Reserved memory: incorrect alignment of CMA region.
> >
> > The following patch to fix this issue, any comments?
> >
> > ======================================================================
> > =================
> >
> > diff --git a/drivers/of/of_reserved_mem.c
> > b/drivers/of/of_reserved_mem.c index 62f467b..a20d4d3 100644
> > --- a/drivers/of/of_reserved_mem.c
> > +++ b/drivers/of/of_reserved_mem.c
> > @@ -124,6 +124,15 @@ static int __init
> __reserved_mem_alloc_size(unsigned long node,
> >                 align = dt_mem_next_cell(dt_root_addr_cells, &prop);
> >         }
> >
> > +       if (of_flat_dt_is_compatible(node,"shared-dma-pool")) {
> > +               phys_addr_t align_required = PAGE_SIZE << max(MAX_ORDER
> - 1, pageblock_order);
> > +                if (!align || align != ALIGN(align, align_required)) {
> > +                        pr_warn("Reserved memory: the alignment not
> set up correctly in '%s' node."
> > +                               "change from %pa to %pa \n", uname,
> &align, &align_required);
> > +                        align = align_required;
> > +                }
> > +        }
> 
> You simply need the max required from the DT (which could be more) or CMA,
> so this can be simplified to:
> 
> align = max(align, PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order));
> 
> I don't think you should warn here either.

If don't want to give some message to tell that the alignment in the DTS specified has been
Updated, the code can be simpler as you said. 


I can submit one patch for this if there is no more comments on this. 

> 
> Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread
* The alignment mismatch issues between the of_reserved_mem and the CMA setup requirement
@ 2015-11-10  3:08     ` Liu Jason
  0 siblings, 0 replies; 9+ messages in thread
From: Liu Jason @ 2015-11-10  3:08 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Rob Herring [mailto:robh+dt at kernel.org]
> Sent: Monday, November 09, 2015 10:26 PM
> To: Liu Hui-R64343
> Cc: devicetree at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; m.szyprowski at samsung.com;
> grant.likely at linaro.org
> Subject: Re: The alignment mismatch issues between the of_reserved_mem
> and the CMA setup requirement
> 
> On Mon, Nov 9, 2015 at 3:39 AM, Liu Jason <Hui.Liu@freescale.com> wrote:
> > There is an alignment mismatch issue between the of_reserved_mem and
> the CMA setup requirement.
> >
> > The alignment in
> > Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> >
> >
> > alignment (optional) - length based on parent's #size-cells
> >                         - Address boundary for alignment of allocation.
> >
> >
> > But this is not exactly match the CMA setup requirement if the
> alignment not set or set it not correctly.
> >
> > The of_reserved_mem will get the alignment from the DTS and pass it to
> > __memblock_alloc_base to do the memory block allocation. If no
> > alignment property in the DTS, the align will be SMP_CACHE_BYTES
> 
> IMO, any alignment requirement in the DTB should reflect h/w alignment
> requirements. We can't know what alignment the OS wants. So if the OS
> needs to further increase the alignment, it should adjust the alignment.
> 

Agree.

> >
> > BUT, The CMA setup require the alignment as the following in the code:
> >
> > align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order)
> >
> > static int __init rmem_cma_setup(struct reserved_mem *rmem) {
> >         phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1,
> pageblock_order);
> >         phys_addr_t mask = align - 1;
> >         unsigned long node = rmem->fdt_node;
> >         struct cma *cma;
> >         int err;
> >
> >         if (!of_get_flat_dt_prop(node, "reusable", NULL) ||
> >             of_get_flat_dt_prop(node, "no-map", NULL))
> >                 return -EINVAL;
> >
> >         if ((rmem->base & mask) || (rmem->size & mask)) {
> >                 pr_err("Reserved memory: incorrect alignment of CMA
> region\n");
> >                 return -EINVAL;
> >         }
> >         <snip>
> > }
> >
> > So, there is very likely that the alignment mismatch between the
> of_reserved_mem and the CMA setup requirement.
> > The sanity check in the rmem_cma_setup will fail and CMA not get set up
> in the end, this is not expected for CMA.
> >
> > In the test, there will be following err log when this mismatch happen.
> >
> > Reserved memory: incorrect alignment of CMA region.
> >
> > The following patch to fix this issue, any comments?
> >
> > ======================================================================
> > =================
> >
> > diff --git a/drivers/of/of_reserved_mem.c
> > b/drivers/of/of_reserved_mem.c index 62f467b..a20d4d3 100644
> > --- a/drivers/of/of_reserved_mem.c
> > +++ b/drivers/of/of_reserved_mem.c
> > @@ -124,6 +124,15 @@ static int __init
> __reserved_mem_alloc_size(unsigned long node,
> >                 align = dt_mem_next_cell(dt_root_addr_cells, &prop);
> >         }
> >
> > +       if (of_flat_dt_is_compatible(node,"shared-dma-pool")) {
> > +               phys_addr_t align_required = PAGE_SIZE << max(MAX_ORDER
> - 1, pageblock_order);
> > +                if (!align || align != ALIGN(align, align_required)) {
> > +                        pr_warn("Reserved memory: the alignment not
> set up correctly in '%s' node."
> > +                               "change from %pa to %pa \n", uname,
> &align, &align_required);
> > +                        align = align_required;
> > +                }
> > +        }
> 
> You simply need the max required from the DT (which could be more) or CMA,
> so this can be simplified to:
> 
> align = max(align, PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order));
> 
> I don't think you should warn here either.

If don't want to give some message to tell that the alignment in the DTS specified has been
Updated, the code can be simpler as you said. 


I can submit one patch for this if there is no more comments on this. 

> 
> Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-11-10  3:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-09  9:39 The alignment mismatch issues between the of_reserved_mem and the CMA setup requirement Liu Jason
2015-11-09  9:39 ` Liu Jason
2015-11-09  9:39 ` Liu Jason
2015-11-09 14:25 ` Rob Herring
2015-11-09 14:25   ` Rob Herring
2015-11-09 14:25   ` Rob Herring
2015-11-10  3:08   ` Liu Jason
2015-11-10  3:08     ` Liu Jason
2015-11-10  3:08     ` Liu Jason

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.