On 08/23/2018 12:45 PM, Simon Glass wrote:
Hi Alex,
On 21 August 2018 at 13:24, Alexander Graf <ag...@suse.de> wrote:
On 21.08.18 19:31, Simon Glass wrote:
Hi Alex,
On 20 August 2018 at 16:29, Alexander Graf <ag...@suse.de> wrote:
On 20.08.18 20:54, Simon Glass wrote:
This partially reverts commit 7e21fbca26d18327cf7cabaad08df276a06a07d8.
That change broke sandbox EFI support for unknown reasons. It also changes
Wouldn't it be better to just figure out the reasons? So far all bugs
I've found were linker script related and quite obvious once you start
to dig into them.
sandbox to use--gc-sections which we don't want.
Why don't we want gc-sections with sandbox?
It is a space optimisation which we don't need for sandbox. It also
complicates the object files unnecessarily.
Put another way, why is it desirable?
The reason we need it for efi_loader is that runtime section code may
generate implicit dependencies on "other data" such as strings, jump
tables, etc that are generated on the fly by the compiler.
The only way to ensure that all these implicit pieces of data are
actually in the runtime section is by forcing function and data sections
onto gcc, because then the implicit pieces of data will inherit the name
tags of the function they're in.
Do they actually not end up in the runtime section without
-fdata-sections, etc.? I would expect that this would just increase
Yes, that's exactly what happens ;). Without data and function sections,
implicit pieces of information go into the big bucket sections
(.data/.rodata/.bss) even when the function they're used in is in an
explicit section.
the size of the runtime section (potentially quite a lot). It seems
wrong for them to be omitted by the toolchain.
Or are you saying that they get picked up by an earlier .lds line, and
so are not available for the (later) runtime section line?
Exactly. Because they're in the generic sections, there is no way to
determine that these variables really are runtime relevant at link time.
For now I am just reverting the sandbox portion as presumably this change
is safe on other architectures.
Sandbox is your target, so you're free to do whatever you like :). But
I'm not sure this is the right path forward. I'd rather like to keep
things consistent.
In what sense?
For efi_loader Runtime Services to actually work we *have* to have
-fdata-sections and -ffunction-sections enabled, otherwise they just
break. So by not enabling them with sandbox we don't get runtime service
support after ExitBootServices (which is useless for sandbox anyway
really), but most importantly we're different from all other targets.
I'm happy to be different from other targets for now. As you say, we
cannot support run-time services for sandbox, at least without some
cunning plan I am not aware of.
So what do you expect happens with this patch? A resend of a patch 1/18
by itself doesn't really tell me what you're trying to say.
The resend was due to me noticing that people did not get the patch on
cc. I only sent this one patch, but I can resend send the whole series
if you like.
I think that it makes sense to differentiate between "this should be
applied for *this* release" and "this is a patch set that as a whole
should be applied".
Or in other words: If the sections really do break sandbox (and again,
for me they don't - I am unable to reproduce still), I would expect that
you send that patch individually as a resend :).
As it stood right now, I simply didn't grasp what your intention was so
I didn't include it in my pull request.
Well I think the revert is needed for this release. For the rest of
the patches, perhaps they should be reviewed and applied when
convenient?
You should be able to repeat the breakage by using my series without
the review (u-boot-dm/efi-working).
Do you see any breakage with current master?
Alex
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
