Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1505490 > unrolled thread

Re: [PATCH v4 2/5] drivers/of: do not add memory for unavailable nodes

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 2/5] drivers/of: do not add memory for unavailable nodes Alistair Popple <apopple@au1.ibm.com> - 2016-10-21 08:30 +0200
    Re: [PATCH v4 2/5] drivers/of: do not add memory for unavailable  nodes Reza Arbab <arbab@linux.vnet.ibm.com> - 2016-10-23 04:00 +0200
    Re: [PATCH v4 2/5] drivers/of: do not add memory for unavailable nodes Michael Ellerman <mpe@ellerman.id.au> - 2016-10-24 12:30 +0200
      Re: [PATCH v4 2/5] drivers/of: do not add memory for unavailable  nodes Reza Arbab <arbab@linux.vnet.ibm.com> - 2016-10-24 20:30 +0200

#1505490 — Re: [PATCH v4 2/5] drivers/of: do not add memory for unavailable nodes

Hi Reza,

On Thu, 6 Oct 2016 01:36:32 PM Reza Arbab wrote:
> Respect the standard dt "status" property when scanning memory nodes in
> early_init_dt_scan_memory(), so that if the node is unavailable, no
> memory will be added.

What happens if a kernel without this patch is booted on a system with some 
status="disabled" device-nodes? Do older kernels just ignore this memory or do 
they try to use it?

From what I can tell it seems that kernels without this patch will try and use 
this memory even if it is marked in the device-tree as status="disabled" which 
could lead to problems for older kernels when we start exporting this property 
from firmware.

Arguably this might not be such a problem in practice as we probably don't 
have many (if any) existing kernels that will boot on hardware exporting these 
properties. However given this patch seems fairly independent perhaps it is 
worth sending as a separate fix if it is not going to make it into this 
release?

Regards,

Alistair

> The use case at hand is accelerator or device memory, which may be
> unusable until post-boot initialization of the memory link. Such a node
> can be described in the dt as any other, given its status is "disabled".
> Per the device tree specification,
> 
> "disabled"
> 	Indicates that the device is not presently operational, but it
> 	might become operational in the future (for example, something
> 	is not plugged in, or switched off).
> 
> Once such memory is made operational, it can then be hotplugged.
> 
> Signed-off-by: Reza Arbab <arbab@linux.vnet.ibm.com>
> ---
>  drivers/of/fdt.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index b138efb..08e5d94 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1056,6 +1056,9 @@ int __init early_init_dt_scan_memory(unsigned long 
node, const char *uname,
>  	} else if (strcmp(type, "memory") != 0)
>  		return 0;
>  
> +	if (!of_flat_dt_device_is_available(node))
> +		return 0;
> +
>  	reg = of_get_flat_dt_prop(node, "linux,usable-memory", &l);
>  	if (reg == NULL)
>  		reg = of_get_flat_dt_prop(node, "reg", &l);
> 

[toc] | [next] | [standalone]

#1506624 — Re: [PATCH v4 2/5] drivers/of: do not add memory for unavailable nodes

Hi Alistair,

On Fri, Oct 21, 2016 at 05:22:54PM +1100, Alistair Popple wrote:
>From what I can tell it seems that kernels without this patch will try 
>and use this memory even if it is marked in the device-tree as 
>status="disabled" which could lead to problems for older kernels when 
>we start exporting this property from firmware.
>
>Arguably this might not be such a problem in practice as we probably 
>don't have many (if any) existing kernels that will boot on hardware 
>exporting these properties.

Yes, I think you've got it right.

>However given this patch seems fairly independent perhaps it is worth 
>sending as a separate fix if it is not going to make it into this 
>release?

Michael,

If this set as a whole is going to miss the release, would it be helpful 
for me to resend 1/5 and 2/5 as a separate set? They are the minimum 
needed to prevent the possible forward compatibility issue Alistair 
describes.

-- 
Reza Arbab

[toc] | [prev] | [next] | [standalone]

#1507078

Alistair Popple <apopple@au1.ibm.com> writes:

> Hi Reza,
>
> On Thu, 6 Oct 2016 01:36:32 PM Reza Arbab wrote:
>> Respect the standard dt "status" property when scanning memory nodes in
>> early_init_dt_scan_memory(), so that if the node is unavailable, no
>> memory will be added.
>
> What happens if a kernel without this patch is booted on a system with some 
> status="disabled" device-nodes? Do older kernels just ignore this memory or do 
> they try to use it?
>
> From what I can tell it seems that kernels without this patch will try and use 
> this memory even if it is marked in the device-tree as status="disabled" which 
> could lead to problems for older kernels when we start exporting this property 
> from firmware.

The code already looks for "linux,usable-memory" in preference to "reg".
Can you use that instead?

That would have the advantage that existing kernels already understand
it.

Another problem with using "status" is we could have device trees out
there that have status = disabled and we don't know about it, and by
changing the kernel to use that property we break people's systems.
Though for memory nodes my guess is that's not true, but you never know ...

cheers

[toc] | [prev] | [next] | [standalone]

#1507560 — Re: [PATCH v4 2/5] drivers/of: do not add memory for unavailable nodes

On Mon, Oct 24, 2016 at 09:24:04PM +1100, Michael Ellerman wrote:
>The code already looks for "linux,usable-memory" in preference to 
>"reg". Can you use that instead?

Yes, we could set the size of "linux,usable-memory" to zero instead of 
setting status to "disabled".

I'll send a v5 of this set which drops 1/5 and 2/5. That would be the 
only difference here.

>That would have the advantage that existing kernels already understand
>it.
>
>Another problem with using "status" is we could have device trees out
>there that have status = disabled and we don't know about it, and by
>changing the kernel to use that property we break people's systems.
>Though for memory nodes my guess is that's not true, but you never know ...

-- 
Reza Arbab

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web