[U-Boot] [RESEND PATCH v9 01/18] Revert "efi_loader: Rename sections to allow for implicit data"

[Alexander Graf]

On 21.08.18 19:31, Simon Glass wrote:
> Hi Alex,
> 
> On 20 August 2018 at 16:29, Alexander Graf <agraf at 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.

> 
>>
>>> 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.

> 
>>
>> 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.


Alex