Re: [XEN v3] xen/arm: Probe the entry point address of an uImage correctly

[Julien Grall <julien@xxxxxxx>]

Hi Ayan,

On 20/12/2022 10:44, Ayan Kumar Halder wrote:
> > > > +
> > > > +    /*
> > > > +     * Currently, we support uImage headers for uncompressed images 
> > > > only.
> > > > +     * Thus, it is valid for the load address and start address to 
> > > > be the
> > > > +     * same. This is consistent with the uboot behavior (Refer
> > > > +     * "case IH_COMP_NONE" in image_decomp().
> > > Please make it clear that you are referring to uboot function.
> > Could we avoid to mention the u-boot function? The reason I am asking 
> > is the code is in a different repo and the function name can easily 
> > change without us noticing (so the comment would rot).
> > Is the behavior documented somewhere in U-boot (or online)? If not, 
> > what's the guarantee that it will not change?
> I could not find any documentation which states this. I found this from 
> the following in uboot source code.
> https://source.denx.de/u-boot/u-boot/-/blob/master/boot/image.c#L458
>
> AFAIU when kernel_uimage_probe() get invoked, the image has already been 
> decompressed. So, I added this as a limitation.
I e looked at the U-boot code and, AFAIU, the check is merely to avoid 
the memcpy() if the image start matches the start of the decompression 
area. So I don't understand why we need the limitation in Xen.
Now the lack of documentation (or at least I didn't find any) makes a 
bit more tricky to understand what the fields in the U-boot header are 
for. I have skimmed through the code and I disagree with the 
implementation you chose for Xen.
The comment for 'ih_ep' suggests this is the entry point address. That's 
may be different from the beginning of your blob. I think this is what 
ih_load is for.
So I think we want to load the blob at ih_load and then set pc to point 
to ih_ep. This will require a bit more work in Xen because the 
assumption so far is the entry point matches the start of the blob.
Please let me known if I missed anything.

Cheers,

--
Julien Grall