[U-Boot] efi_loader: runtime services implementation broken?

[Alexander Graf]

On 03.07.18 21:39, Heinrich Schuchardt wrote:
> On 07/03/2018 06:51 PM, Bin Meng wrote:
>> Hi Alex,
>>
>> On Tue, Jul 3, 2018 at 11:05 PM, Alexander Graf <agraf at suse.de> wrote:
>>> On 07/03/2018 04:56 PM, Bin Meng wrote:
>>>>
>>>> Hi Alex,
>>>>
>>>> On Tue, Jul 3, 2018 at 10:32 PM, Alexander Graf <agraf at suse.de> wrote:
>>>>>
>>>>> On 07/03/2018 04:24 PM, Bin Meng wrote:
>>>>>>
>>>>>> Hi Alex, Heinrich,
>>>>>>
>>>>>> At present not all interfaces in lib/efi_loader/efi_runtime.c are
>>>>>> declared as __efi_runtime. But only declaring those as __efi_runtime
>>>>>> is not enough. The data these interfaces use should be declared as
>>>>>> __efi_runtime_data too. More worse, any U-Boot APIs called by these
>>>>>> interfaces should be __efi_runtime and __efi_runtime_data
>>>>>> respectively.
>>>>>
>>>>>
>>>>> Correct. This is done for everything right now, no?
>>>>
>>>> For example, efi_set_virtual_address_map() is not declared as
>>>> __efi_runtime.
>>>
>>>
>>> Is that called post exit boot services on x86?
>>
>> Yes, if you look at efi_runtime_services structure in
>> lib/efi_loader/efi_runtime.c, you can see
>> efi_set_virtual_address_map() is hooked up there, and all interfaces
>> in efi_runtime_services are supposed to be called by OS as their names
>> indicate, runtime not bootime.
> 
> No, this not correct. Runtime functions may be called before and after
> ExitBootServices. Otherwise the EFI shell would not be able to read
> variables for instance.
> 
> The SetVirtualAdressMap service is only called after ExitBootServices.
> At that time the caller is the owner of the memory map. So this function
> must be marked as __efi_runtime.
> 
> Our current coding makes the invalid assumption that U-Boot is owner of
> the memory until SetVirtualAddressMap() is called.
> 
> Other deficiencies are that we do not provide the ConvertPointer()
> service and do not raise notify an event when SetVirtualAddressMap() is
> called.
> 
>>
>>>
>>>>
>>>>>> Eventually we need mark the RAM where the whole U-Boot image lives as
>>>>>> runtime service code and data and end up leaving whole U-Boot image in
>>>>>> the RAM after kernel boots?
>>>>>
>>>>>
>>>>> Why?
>>>>>
>>>> Unless we track down every APIs called by runtime services and mark
>>>> them correctly as __efi_runtime code/data.
>>>
>>>
>>> That's the idea, yes. The vector should be quite small as long as we don't
>>> actually implement / reuse drivers.
>>
>> However this currently seems not to be the case ..
>>
>>>
>>>>
>>>>>> Right now I am testing booting Linux on Intel Galileo with 'bootefi'
>>>>>> and kernel just hangs during the boot. Initial debugging shows that
>>>>>> kernel crashes when calling runtime service
>>>>>> efi_set_virtual_address_map().
>>>>>
> 
> Is this a regression or did it never work?

It's only ever been tested on ARM, so maybe the x86 Linux EFI stub
behaves differently. If set_virtual_address_map needs to be intact after
exiting boot services, we need to move all of the relocation information
and functionality into runtime sections as well.


Alex