On Friday 04 August 2017 03:44 PM, Michael Ellerman wrote:
Hari Bathini <hbath...@linux.vnet.ibm.com> writes:

On Friday 04 August 2017 09:21 AM, Michael Ellerman wrote:
Hari Bathini <hbath...@linux.vnet.ibm.com> writes:

As linux,memory-limit node is set and also later used by the kernel,
avoid endian conversions for this property.

Fixes: 493adffcb43f ("powerpc: Make prom_init.c endian safe")
Cc: sta...@vger.kernel.org # 3.12+
Cc: Anton Blanchard <an...@ozlabs.org>
Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
Signed-off-by: Hari Bathini <hbath...@linux.vnet.ibm.com>
---
   arch/powerpc/kernel/prom_init.c |    3 +--
   1 file changed, 1 insertion(+), 2 deletions(-)
As Ben said, this is not OK. The flat device tree is a data
structure with a specified format[1], we don't violate the spec just to
avoid an endian swap.

Is there an actual bug you're trying to solve?
Yep. While retrieving this property in prom.c, no endian conversion is
being done.
It was broken for a while. Let me do the endian swap in prom.c while
retrieving..
Does it actually not work though, mem=x on the command line?

mem=X works fine. The problem is with the early cmdline parsing of
'mem=' in prom_init, which is treating fadump_reserve_mem=X as
mem=X. So, when fadump_reserve_mem=X is passed, endian swapped
version of X is set to memory_limit as early parser takes it for mem=X
and linux,memory-limit read is not endian safe currently. This bug
was not hit so far as prom_memory_limit is set only when X is
< ram_top && > alloc_bottom which is not the case generally.

I think that code in prom.c is basically dead code, it's still there
because we were afraid removing it would break something. These days we
parse the command line early enough that we don't need those properties.



This problem is not seen with mem=X as memory_limit is overwritten
with the right value as soon as parse_early_param() is called in prom.

Should I just get rid of linux,memory-limit node and mem=X handling
from early_cmdline_parse() in prom_init as this has been broken for
a while and nobody seem to have had a problem with that?

Thanks
Hari

Reply via email to