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

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.
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?
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().
Regards, Bin

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?
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?
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().
Can you please try with my efi-next branch? I added a few patches that allow gcc to put implicit data and code into the runtime section. Without those, you may get constant propagation into a the common .rodata segment for example.
Alex

Hi Alex,
On Tue, Jul 3, 2018 at 10:32 PM, Alexander Graf agraf@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.
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.
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().
Can you please try with my efi-next branch? I added a few patches that allow gcc to put implicit data and code into the runtime section. Without those, you may get constant propagation into a the common .rodata segment for example.
Still the same.
Regards, Bin

On 07/03/2018 04:56 PM, Bin Meng wrote:
Hi Alex,
On Tue, Jul 3, 2018 at 10:32 PM, Alexander Graf agraf@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?
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.
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().
Can you please try with my efi-next branch? I added a few patches that allow gcc to put implicit data and code into the runtime section. Without those, you may get constant propagation into a the common .rodata segment for example.
Still the same.
Ok, maybe the x86 efi stub does things different from the ARM one then.
Alex

Hi Alex,
On Tue, Jul 3, 2018 at 11:05 PM, Alexander Graf agraf@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@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.
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().
Can you please try with my efi-next branch? I added a few patches that allow gcc to put implicit data and code into the runtime section. Without those, you may get constant propagation into a the common .rodata segment for example.
Still the same.
Ok, maybe the x86 efi stub does things different from the ARM one then.
maybe :(
Regards, Bin

On 07/03/2018 06:51 PM, Bin Meng wrote:
Hi Alex,
On Tue, Jul 3, 2018 at 11:05 PM, Alexander Graf agraf@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@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?
Best regards
Heinrich
Can you please try with my efi-next branch? I added a few patches that allow gcc to put implicit data and code into the runtime section. Without those, you may get constant propagation into a the common .rodata segment for example.
Still the same.
Ok, maybe the x86 efi stub does things different from the ARM one then.
maybe :(
Regards, Bin

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

From: Alexander Graf agraf@suse.de Date: Tue, 3 Jul 2018 22:08:05 +0200
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.
The OpenBSD EFI bootloader calls ExitBootServices() before handing control to the kernel. The OpenBSD/arm64 kernel then calls SetVirtualAddressmap(). This works, but probably because it happens very early when the kernel is only using memory inside a 32M block that has been allocated by the bootloader.
So yes, I think that functionality needs to marked as __efi_runtime.
Cheers,
Mark

Hi Heinrich,
On Wed, Jul 4, 2018 at 3:39 AM, Heinrich Schuchardt xypron.glpk@gmx.de 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@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@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.
Yes, I meant to say runtime services interfaces should be available after ExitBootServices, and that's for runtime like OS to call.
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?
I believe it never worked on x86.
Regards, Bin
participants (4)
-
Alexander Graf
-
Bin Meng
-
Heinrich Schuchardt
-
Mark Kettenis