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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 16 Sep 2020, Julien Grall 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?

Yeah, ignoring a bank with size=0 is fine. I checked the epapr and it
doesn't specify that size=0 is invalid, so I think it is actually better
to ignore it and continue even from a spec perspective.