On 21.02.2019, at 13:00, Andre Przywara andre.przywara@arm.com wrote:
On Thu, 21 Feb 2019 12:47:04 +0100 Alexander Graf agraf@suse.de wrote:
Hi Alex,
On 02/21/2019 02:30 AM, Andre Przywara wrote:
Read the specified "arch" value from a legacy or FIT U-Boot image and store it in our SPL data structure. This allows loaders to take the target architecture in account for custom loading procedures. Having the complete string -> arch mapping for FIT based images in the SPL would be too big, so we leave it up to architectures (or boards) to overwrite the weak function that does the actual translation, possibly covering only the required subset there.
Signed-off-by: Andre Przywara andre.przywara@arm.com
Reviewed-by: Alexander Graf agraf@suse.de
Thanks for having a look!
I don't fully buy the argument that the generic mapping would be too big though. Realistically you should be able to get away with 1 or 2 branches per case, no? So that would be maybe 40 instructions?
You are apparently not in the Allwinner SPL mindset ;-)
I am sometimes struggling against the 192KB limit on the RK3399. So while my current problems (SPL on the A31 was a different story) may sound like “luxury” to you, I do understand the pain.
I get excited when I find a way of saving 100 bytes (see the writel_relaxed patch), so wasting 160 bytes when we will probably never need to return IH_ARCH_X86_64 doesn't sound right to me.
And Philipp is right: the canonical way would be to use uimage_arch[] from common/image.c, which is quite big.
We could have a big #ifdef cascade, something like: +#ifdef CONFIG_ARM
- if (!strcmp(arch_str, "arm"))
return IH_ARCH_ARM;- if (!strcmp(arch_str, "arm64"))
return IH_ARCH_ARM64;+#endif +#ifdef CONFIG_X86
- if (!strcmp(arch_str, , "x86"))
return IH_ARCH_I386;- if (!strcmp(arch_str, "x86_64"))
return IH_ARCH_X86_64;+#endif
Could we have a preprocessor-guarded table of the form ONLY_ON(ARM)( { “arm”, IH_ARCH_ARM }, ) ONLY_ON(ARM)( { “arm64”, IH_ARCH_ARM64 }, ) for this?
Then the function would be for ( …, curr_element = …, ... ) { if (!strcmp(arch_str, curr_element.arch_str)) return curr_element.ih_value; which should be less than the hypothetical 40 instructions, I would hope.
Then again, I didn’t send this through a compiler to look at the assembly generated. And I have been surprised by code-size on AArch64 before.
I usually am in favor of cleaning up the common code paths, so my personal preference would be in figuring out a way how we can change uimage_arch[] to become useful in a space-constrained SPL context.
Phil.