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

[Bin Meng]

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

[Alexander Graf]

Post by Bin Meng
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?

Post by Bin Meng
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?

Post by Bin Meng
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

[Bin Meng]

Hi Alex,

Post by Alexander Graf

Post by Bin Meng
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.

Post by Alexander Graf

Post by Bin Meng
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.

Post by Alexander Graf

Post by Bin Meng
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

[Alexander Graf]

Post by Bin Meng
Hi Alex,

Post by Alexander Graf

Post by Bin Meng
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?

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng
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.

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng
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

[Bin Meng]

Hi Alex,

Post by Alexander Graf

Post by Bin Meng
Hi Alex,

Post by Alexander Graf

Post by Bin Meng
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.

Post by Alexander Graf

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng
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 ..

Post by Alexander Graf

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng
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

[Heinrich Schuchardt]

Post by Bin Meng
Hi Alex,

Post by Alexander Graf

Post by Bin Meng
Hi Alex,

Post by Alexander Graf

Post by Bin Meng
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.

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng
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 ..

Post by Alexander Graf

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng
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

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng

Post by Alexander Graf
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

[Alexander Graf]

Post by Heinrich Schuchardt

Post by Bin Meng
Hi Alex,

Post by Alexander Graf

Post by Bin Meng
Hi Alex,

Post by Alexander Graf

Post by Bin Meng
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.

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng
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 ..

Post by Alexander Graf

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng
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

[Mark Kettenis]

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

[Bin Meng]

Hi Heinrich,

Post by Heinrich Schuchardt

Post by Bin Meng
Hi Alex,

Post by Alexander Graf

Post by Bin Meng
Hi Alex,

Post by Alexander Graf

Post by Bin Meng
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.

Post by Heinrich Schuchardt
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.

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng
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 ..

Post by Alexander Graf

Post by Bin Meng

Post by Alexander Graf

Post by Bin Meng
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