Re: DT with memory bank of size 0 (WAS: Re: AW: AW: Colibri imx8qxp: Missing kernel boot module)

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

> On 16 Sep 2020, at 18:30, Julien Grall <julien@xxxxxxx> wrote:
> 
> On 14/09/2020 15:26, Daniel Wagner2 wrote:
>> Hi Julien,
> 
> Hi Daniel,
> 
> I am moving the thread to xen-devel and adding a couple of more folks.
> 
>>> 
>>>> 
>>>> this is the full version of the fdt that threw the error:
>>>> https://pastebin.com/63TZ9z3k
>>>> The problematic memory node appears in line 126
>>> 
>>> Thanks! The output looks corrupted as some of the lines are not valid DTB:
>>> 
>>> fsl,pins = * 0x000000009300184c [0x00000048];
>>> 
>>> Although, I am not sure if it is just U-boot dumping the DTB differently.
>>> 
>>> Anyway, after removing the "corrupted" line, I managed to get a compile
>> the
>>> DTB. I don't have a Colibri IMX8QXP. However, given this is an early
>> parsing
>>> error, I have just embed the DTB in Xen binary via CONFIG_DTB_FILE.
>>> 
>>> Unfortunately I couldn't reproduce your error. This either suggests the
>> DTB gets
>>> corrupted or Xen doesn't access the DTB with the correct memory attribute.
>>> 
>>> Do you have the DTB in hand?
>> Sorry for the corrupted version, I've uploaded the DTB
>> (fsl-imx8qxp-colibri-eval-v3.dtb) to
>> https://drive.google.com/drive/folders/1jbpnz35sC0NbCyEjrkLqelBsKBztW1S6?usp
>> =sharing
>> I have also uploaded my modified xen source files.
>> 1. arch/arm/bootfdt.c
>> where I have added the additional printk's seen in the log and
>> 2. arch/arm/setup.c
>> where I rerun the devicetree parser in line 935 to get the logs, since the
>> console is not yet initialised when the function is called for the first
>> time and I
>> didn't manage to enable earlyprintk.
>> I think the breaking point is the second memory bank which appears in the
>> logs (see the output line marked with "!!")  with start=0x8 8000 0000 and
>> size=0.
>> It isn't specified in the DTB, so I am not sure where this comes from.
>> It has size=0 so
>> if ( !size )
>>     {
>>         printk("invalid size, bank %d\n",i);
>>         return -EINVAL;
>>     }
>> In bootfdt.c makes the function stop.
>> Log:
>> (XEN) arch/arm/bootfdt.c: early_scan_node
>> (XEN) -> fdt: node `memory@80000000': parsing
>> (XEN) -> process_memory_node
>> (XEN)
>> (XEN) arch/arm/bootfdt.c: process_memory_node
>> (XEN) ->found memory:reg
>> (XEN) ->cell=
>> (XEN) ->banks=2
>> (XEN) ->mem->nr_banks=1
>> (XEN) ->NR_MEM_BANKS=128
>> (XEN) ->start=0x80200000 size=0x7fe00000
>> !! (XEN) ->start=0x880000000 size=0
>> (XEN) invalid size, bank 1
>> (XEN) END of arch/arm/bootfdt.c: process_memory_node
> 
> When I tried to run it on the model I get:
> 
> (XEN) device_tree_for_each_node: memory@80000000
> (XEN)
> (XEN) arch/arm/bootfdt.c: early_scan_node
> (XEN) -> fdt: node `memory@80000000': parsing
> (XEN) -> process_memory_node
> (XEN)
> (XEN) arch/arm/bootfdt.c: process_memory_node
> (XEN) ->found memory:reg
> (XEN) ->cell=
> (XEN) ->banks=1
> (XEN) ->mem->nr_banks=0
> (XEN) ->NR_MEM_BANKS=128
> (XEN) ->start=0x80000000 size=0x40000000
> (XEN) END of arch/arm/bootfdt.c: process_memory_node
> 
>> Btw 8_8000_0000 is the start address of this systems DDR Main memory,
>> according to the Reference Manual of the i.MX8QXP.
> I couldn't find this value in the DT. It is possible that U-boot is modifying 
> the memory node before jumping to Xen (or Linux).
> 
> Looking at Linux, they seem to ignore any bank with size == 0. I am starting 
> to wonder whether your DT is (ab)using it.
> 
> Do you have Linux running on baremetal on this board? If so would you mind to 
> dump the DT from the userspace (via /proc/device-tree) this time?
> 
> In any case, we may want to relax the check in Xen. Any opinions?

It would make sense to ignore entries with size 0 even more if Linux is already 
doing it.

Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall