Add UEFI image loader for arm64. #75

jongwu · 2021-04-09T09:15:23Z

UEFI image for virtual platform on arm64 is not formatted,
so I add a specific loader for it here.

Signed-off-by: Jianyong Wu jianyong.wu@arm.com

@aghecenco @andreeaflorescu

andreeaflorescu

I don't really know much about UEFI. Do you also have an example of how this is used?

Can you also add some tests?

andreeaflorescu · 2021-04-13T07:39:56Z

src/loader/aarch64/pe/mod.rs

+/// uefi image for virtual machine on arm64 is not formatted, so
+/// a special loader for it is needed.


I don't think this needs to be part of the public interface documentation.

ok, I will remove it.

andreeaflorescu · 2021-04-13T07:45:51Z

src/loader/aarch64/pe/mod.rs

    }
 }

+/// load uefi for aarch64


nit: can you format the comment similar to how other comments are formatted: Load UEFI image in guest memory.

michael2012z · 2021-04-13T08:36:32Z

src/loader/aarch64/pe/mod.rs

+/// * `guest_addr` - The offset in guest_mem from which the UEFI image will be loaded.
+/// * `uefi_image` - The UEFI image
+#[cfg(target_arch = "aarch64")]
+pub fn load_uefi<F, M: GuestMemory>(


The code of this functions is rather similar to load_dtb, can you refactor to reduce some duplicate code?

jongwu · 2021-04-13T09:10:21Z

@andreeaflorescu -, Thanks for reply.
Yeah, It's fine to use this UEFI image loader even having little knowledge about it. You can simply take UEFI image like a kernel image and load it. As the kernel image format is different from the UEFI image on arm64, we need some twists on the kernel loader. Roughly, remove the header check and set the entry point of UEFI image as its very beginning then we get a new loader. That is what this patch does. So easy isn't? Here is an example of how to use it:
let entry_point = linux_loader::loader::pe::load_uefi(
mem.deref(), // Guest memory where UEFI image should be loaded
GuestAddress(arch::get_uefi_start()), // The offset in guest memory from which the UEFI image will be loaded
&uefi_image,
)
we load the uefi_image to mem.deref() at GuestAddress(arch::get_uefi_start()) and get the entry point in "entry_point", then we can set the PC in guest as entry_point and begin to run.

rbradford · 2021-04-13T09:47:17Z

For loading a UEFI/OVMF on Cloud Hypervisor with x86-64 we used the PVH (like Xen) support so that we didn't need to modify linux-loader or the VMM. I see that ArmVirtPkg also supports Xen: https://github.com/tianocore/edk2/blob/master/ArmVirtPkg/ArmVirtXen.fdf

If you can make that work then I think that would a a cleaner approach.

rbradford · 2021-04-13T09:49:39Z

For loading a UEFI/OVMF on Cloud Hypervisor with x86-64 we used the PVH (like Xen) support so that we didn't need to modify linux-loader or the VMM. I see that ArmVirtPkg also supports Xen: https://github.com/tianocore/edk2/blob/master/ArmVirtPkg/ArmVirtXen.fdf

If you can make that work then I think that would a a cleaner approach.

Oh, I think i misunderstood. This is a loader for the UEFI PE format rather than for a UEFI firmware blob. What's your use case for this?

jongwu · 2021-04-13T10:14:11Z

Hello @rbradford -, for now, the UEFI image on arm64 virtual platform (eg, qemu, kvmtool) is not formatted, not a pe or elf. Unfortunately, we have no such a loader to load a file without header in it.
In my case, I want to introduce UEFI support in cloud-hypervisor on arm64, but I can't load the UEFI as there is lack of a suitable loader. That's why I want to introduce a new one here.

UEFI image for virtual platform on arm64 is not formatted, so we add a specific loader for it here. Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu · 2021-04-19T01:43:55Z

Hi @andreeaflorescu -, any comments?
I have updated this patch, adding a test for load_uefi and amending the annotation.

andreeaflorescu · 2021-04-19T06:10:37Z

Hi @andreeaflorescu -, any comments?
I have updated this patch, adding a test for load_uefi and amending the annotation.

Thanks, I can take another look in a couple of days. Can you share an example of how this is used in practice? Do you happen to have a patch that adds this support in Cloud Hypervisor?

@rbradford since it looks like this support is going to end up in Cloud Hypervisor, can you also please take a look at this PR?

michael2012z · 2021-04-19T15:28:11Z

@andreeaflorescu , there is a PR in Cloud-hypervisor that includes the WIP commits for UEFI & ACPI support for AArch64.

The code to call this new function is like this: cloud-hypervisor/cloud-hypervisor@9742ecd#diff-9c91c6326e9d86ef107a59aed3f13a9bb91790f9f4be1668d22cda12a28bc28cR905

The UEFI binary is header-less, it's impossible to recognize it by format.
So the logic is: At first try to load the binary as PE; if the format is wrong, try to load it as UEFI.

jongwu · 2021-06-02T08:59:43Z

Hello @andreeaflorescu -, could you please take a look of this patch? We have a PR in Cloud Hypervisor to enable Acpi on arm64 which depends on this patch. Also, you can find the use case of the new handler added in this patch here

We have blocked here so long, thus I'm very appreciate if there is any progress on this PR. Thanks!

andreeaflorescu · 2021-06-02T12:27:56Z

@jongwu I can only provide high level feedback, and that might not be very meaningful. Can someone from Cloud Hypervisor review this since it is going to be used there? You should not block on my approval as I really do not have enough context.

rbradford

It looks like it doesn't do much more than read the file into guest memory. Maybe it makes sense to just do that in CH?

jongwu · 2021-06-03T01:29:09Z

Thanks @rbradford @andreeaflorescu -, since we can do this change in CLH, I will close this.

jongwu force-pushed the uefi branch from e2fec4e to 648166e Compare April 9, 2021 09:53

andreeaflorescu reviewed Apr 13, 2021

View reviewed changes

michael2012z reviewed Apr 13, 2021

View reviewed changes

Add UEFI image loader for arm64.

1af423f

UEFI image for virtual platform on arm64 is not formatted, so we add a specific loader for it here. Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu force-pushed the uefi branch from 648166e to 1af423f Compare April 14, 2021 05:12

MrXinWang mentioned this pull request May 26, 2021

ACPI initial support on AArch64 cloud-hypervisor/cloud-hypervisor#2241

Closed

michael2012z mentioned this pull request May 31, 2021

Enable ACPI on AArch64 cloud-hypervisor/cloud-hypervisor#2680

Merged

rbradford reviewed Jun 2, 2021

View reviewed changes

jongwu closed this Jun 3, 2021

		/// uefi image for virtual machine on arm64 is not formatted, so
		/// a special loader for it is needed.

Add UEFI image loader for arm64. #75

Add UEFI image loader for arm64. #75

Uh oh!

Conversation

jongwu commented Apr 9, 2021

Uh oh!

andreeaflorescu left a comment

Choose a reason for hiding this comment

Uh oh!

andreeaflorescu Apr 13, 2021

Choose a reason for hiding this comment

Uh oh!

jongwu Apr 13, 2021

Choose a reason for hiding this comment

Uh oh!

andreeaflorescu Apr 13, 2021

Choose a reason for hiding this comment

Uh oh!

jongwu Apr 13, 2021

Choose a reason for hiding this comment

Uh oh!

michael2012z Apr 13, 2021

Choose a reason for hiding this comment

Uh oh!

jongwu commented Apr 13, 2021

Uh oh!

rbradford commented Apr 13, 2021

Uh oh!

rbradford commented Apr 13, 2021

Uh oh!

jongwu commented Apr 13, 2021

Uh oh!

jongwu commented Apr 19, 2021

Uh oh!

andreeaflorescu commented Apr 19, 2021

Uh oh!

michael2012z commented Apr 19, 2021

Uh oh!

jongwu commented Jun 2, 2021

Uh oh!

andreeaflorescu commented Jun 2, 2021

Uh oh!

rbradford left a comment

Choose a reason for hiding this comment

Uh oh!

jongwu commented Jun 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants