RPi4's memreserve use in fdt not handled by aarch64 fdt_get_reserved_mem (called via its initarm)

[Mark Millard]

On 2020-Feb-9, at 22:00, Mark Millard <marklmi at yahoo.com> wrote:

> On 2020-Feb-9, at 21:12, Mark Millard <marklmi at yahoo.com> wrote:
> 
>> aarch64 seems to be ignoring the RPi4B's memreserve use.
>> 
>> My hypothesis is that head -r356776 and later are
>> allocating RAM areas that overlap with the
>> memreserve area.
>> 
>> 1st see separate submittals about the live dts text reported
>> by fdt print in u-boot on the example RPi4B 4 GiByte machine:
>> 
>> https://lists.freebsd.org/pipermail/freebsd-arm/2020-February/021207.html
>> https://lists.freebsd.org/pipermail/freebsd-arm/2020-February/021205.html
>> 
>> Then:
>> 
>> aarch64's initarm uses fdt_get_reserved_mem and not
>> fdt_get_reserved_regions. So, looking at what that
>> implies:
>> 
>> int
>> fdt_get_reserved_mem(struct mem_region *reserved, int *mreserved)
>> {
>>       pcell_t reg[FDT_REG_CELLS];
>>       phandle_t child, root;
>>       int addr_cells, size_cells;
>>       int i, rv;
>> 
>>       root = OF_finddevice("/reserved-memory");
>>       if (root == -1) {
>>               return (ENXIO);
>>       }
>> . . . (I'll not list it all to show the lack of
>> memreserve handling) . . .
>> 
>> This does not check for and handle memreserve. 
>> 
>> By contrast armv7 and armv6 each have an initarm
>> that uses fdt_get_reserved_regions and that in
>> turn has:
>> 
>> int
>> fdt_get_reserved_regions(struct mem_region *mr, int *mrcnt)
>> {
>>       pcell_t reserve[FDT_REG_CELLS * FDT_MEM_REGIONS];
>>       pcell_t *reservep;
>>       phandle_t memory, root;
>>       int addr_cells, size_cells;
>>       int i, res_len, rv, tuple_size, tuples;
>> 
>>       root = OF_finddevice("/");
>>       memory = OF_finddevice("/memory");
>>       if (memory == -1) {
>>               rv = ENXIO;
>>               goto out;
>>       }
>> 
>>       if ((rv = fdt_addrsize_cells(OF_parent(memory), &addr_cells,
>>           &size_cells)) != 0)
>>               goto out;
>> 
>>       if (addr_cells > 2) {
>>               rv = ERANGE;
>>               goto out;
>>       }
>> 
>>       tuple_size = sizeof(pcell_t) * (addr_cells + size_cells);
>> 
>>       res_len = OF_getproplen(root, "memreserve");
>>       if (res_len <= 0 || res_len > sizeof(reserve)) {
>>               rv = ERANGE;
>>               goto out;
>>       }
>> 
>>       if (OF_getprop(root, "memreserve", reserve, res_len) <= 0) {
>>               rv = ENXIO;
>>               goto out;
>>       }
>> . . .
>> 
>> So this handles memreserve but not /reserved-memory .
>> 
>> It appears that for the RPi4B's the 32-bit "normal use"
>> has lead to aarch64 having memreserve instead of
>> /reserved-memory .
>> 
> 
> I managed to make a quick test patch for head -r356529
> but it did not make the RPi4B boot attempt behave
> differently. So, either I messed up or handling memreserve
> is not sufficient. (Some alternate information might be
> covering the address range already?)

Example alternate information was:

        memory at 0 {
                device_type = "memory";
                reg = <0x00000000 0x00000000 0x3b400000 0x00000000 0x40000000 0xbc000000>;
        };

That looks like it avoids indicating anything from the
memreserve region. (I've no clue if such is a requirement
for memory at 0, but it happens to be the case for the test
context.)

> I'm not familiar with the subject matter in the code, so
> I may have messed up the test. I just used:
> 
> # svnlite diff /usr/src/sys/dev/fdt/fdt_common.c
> Index: /usr/src/sys/dev/fdt/fdt_common.c
> ===================================================================
> --- /usr/src/sys/dev/fdt/fdt_common.c	(revision 357529)
> +++ /usr/src/sys/dev/fdt/fdt_common.c	(working copy)
> @@ -512,6 +512,11 @@
> 
> 	root = OF_finddevice("/reserved-memory");
> 	if (root == -1) {
> +		// Fail over to checking for and handling memreserve,
> +		// such as for a RPi4B.
> +		if (0 == fdt_get_reserved_regions(reserved,mreserved))
> +			return (0);
> +
> 		return (ENXIO);
> 	}

I do not know if such is appropriate code or not. For my
specific test context, it makes no difference.


===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)