Hi Julien,
I need a clarification.
On 20/12/2022 12:51, Ayan Kumar Halder wrote:
On 20/12/2022 12:14, Julien Grall wrote:
Hi Ayan,
Hi Julien,
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.
I think you are correct. I will rework this to correctly handle load
address and entry point.
Is it fine to keep these two constraints ?
/*
* While uboot considers 0x0 to be a valid load/start address, for Xen
* to mantain parity with zimage, we consider 0x0 to denote position
* independent image. That means Xen is free to load such an image at
* any valid address.
* Thus, we will print an appropriate message.
*/
if ( info->zimage.start == 0 )
printk(XENLOG_INFO
"No load address provided. Xen will decide where to load
it.\n");
/*
* If the image supports position independent execution, then user
cannot
* provide an entry point as Xen will load such an image at any
appropriate
* memory address. Thus, we need to return error
*/
if ( (info->zimage.start == 0) && (info->entry != 0) )
{
printk(XENLOG_ERROR
"Entry point cannot be non zero for PIE image.\n");
return -EINVAL;
}
- Ayan
- Ayan
Cheers,
