Hi Rasmus
On Tue, 2023-10-24 at 16:32 +0200, Rasmus Villemoes wrote:
> On 24/10/2023 15.15, Marcel Ziswiler wrote:
>
> Hi Marcel
>
> tl;dr: can you try
>
> @@ -330,7 +335,7 @@ static int spl_romapi_load_image_stream(struct
> spl_image_info *spl_image,
>
> ss.base = phdr;
> ss.end = p;
> - ss.pagesize = pagesize;
> + ss.pagesize = 1024;
>
> memset(&load, 0, sizeof(load));
> load.bl_len = 1;
>
> and also put 1024 in the binman node so u-boot.itb is also 1024-byte
> aligned?
That did not help.
> This will likely break usb boot (where pagesize==1), so if you want the
> binary to work with usb you can make the RHS instead "(pagesize > 1 ?
> 1024 : 1)" or something like that.
>
> I don't think it will work, but OTOH my analysis below doesn't find any
> other (fundamental) difference between the old and new code.
>
> >
> > On Tue, 2023-10-24 at 13:17 +0200, Rasmus Villemoes wrote:
> >
> > >
> > > and this works just fine. But in my case ss->pagesize is 1, whereas you
> > > have 512.
> > >
> > > Just exactly how are you booting? It says "Boot Stage: Primary boot"
> > > whereas I'm doing serial download with uuu (i.e. "Boot Stage: USB boot").
> >
> > Yes, regular eMMC boot.
>
> [snip]
>
> >
> > At least I am not aware that we would be doing anything special for eMMC
> > boot.
>
> Well, I/we usually do boot from eMMC (except during bootstrap where we
> use uuu), more precisely from the eMMC boot partitions, but that doesn't
> end up using the spl_romapi_load_image_stream() but instead the expected
> spl_romapi_load_image_seekable().
>
> Perhaps some NXP folks can explain the logic behind
>
> static int is_boot_from_stream_device(u32 boot)
> {
> u32 interface;
>
> interface = boot >> 16;
> if (interface >= BT_DEV_TYPE_USB)
> return 1;
>
> if (interface == BT_DEV_TYPE_MMC && (boot & 1))
> return 1;
>
> return 0;
> }
>
> Apparently that "boot & 1" is an indication that "eMMC fast boot mode"
> is enabled (not to be confused with fastboot which is a USB protocol
> that only becomes relevant once U-Boot proper is up and running).
Yes, we use eMMC fast boot mode.
> As I've complained about previously, the ROM API is almost entirely
> undocumented. There's something in imx93rm.pdf (which is where I have
> that eMMC fast boot info from), but I have no idea if that applies to
> imx8/imx8mp as well.
>
> Are there any alignment requirements on the destination and/or the size
> parameter? Is there some timing requirements so that if we read too late
> the data is lost? So could e.g. a bunch of printf debugging break boot?
>
> Because I really can't see the fundamental difference between now and
> then. Both before and after the commit in question, the code does:
>
> - read 1024 bytes at a time, search each chunk for FDT magic
>
> This then gives the "Find img info 0x4802b000"
>
> - make sure the leftover within that 1024 chunk is a full FDT header
> (usually ok, so no output)
>
> - read the FDT size from that header, round up to a multiple of 1024,
> fetch that - that's the "Need continue download 1024".
>
> And then the flow deviates.
>
> The old code would do the fake spl_fit_load to figure out the maximum
> offset (thus the real total .itb size), then take that remaining size,
> round up to ->pagesize (512), and fetch that with one final
> rom_api_download_image(p, 0, imagesize);.
>
> My code instead does a number of rom_api_download_image() calls, where
> each size argument is rounded up to the ->pagesize from the boot_info
> query (i.e. 512). So it's possible that for one of those calls, the
> destination is only 512-byte aligned (because a previous call fetched an
> odd number of 512 byte blocks), and that doesn't happen in the previous
> case where all but the last rom_api_download_image() have fetched 1024
> byte aligned chunks.
>
> What am I missing?
Good question. Some more debugging revealed that we are missing 464 bytes at
the beginning of the buffer. Why
would that be?
> Rasmus
Cheers
Marcel
