This is the approach I strongly prefer to D31065, fix the root of the problem. I don't how often hardware clocks are on ARM64 UEFI systems, but such could be present. In which case being able to access the clock inside Xen would be useful.

Since I lack such hardware, this is untested beyond it compiles. This is also sort of fallout from D28619.

I don't know whether machdep.bootmethod is worth preserving, but I could see installation scripts wanting it. I doubt reporting "XEN" triggers a special case for such. I'm unsure whether it is even worth having FW_HYPER since this starts overlapping with "vm_guest".

I kinda like this, but kinda don't... IT seems to conflate too many things together into firmware.

I agree to an extent, but I feel the distinction isn't worth preserving. Notably if there was a real issue with this there should already be far more values for the variables involved, yet those don't exist.

"bootmethod" and "arm64_bus_method" only have 3 defined states. Since there is now a driver which needs to be made machine-independent (sys/dev/xen/efi/pvefi.c), merging these together is now quite desirable.

This is confusing two mostly independent concepts. One is the firmware type. On arm64 it is almost always UEFI, however it could also boot via the same ABI as a Linux kernel. The other is the hardware description method.

On amd64 there is already 'efi_boot' var.

In D31955#720853, @andrew wrote:

This is confusing two mostly independent concepts. One is the firmware type. On arm64 it is almost always UEFI, however it could also boot via the same ABI as a Linux kernel. The other is the hardware description method.

Yes, they are somewhat independent, but they are interrelated and strongly correlate.

Of the spots touched by this they all appear to ask one of two questions:

1. Use OpenFirmware device-tree or ACPI table?
2. Are UEFI runtime services available?

I'm under the impression using device-trees pretty well excludes full UEFI runtime services. The only case where these mix at all is U-Boot's UEFI support, which mostly appears to be good enough for bootloader use and only minimally implement the UEFI spec.
As such this seems to be a spectrum with device-trees on one end and full UEFI the other.

In D31955#720857, @kib wrote:

On amd64 there is already 'efi_boot' var.

That actually concerns me. So AMD64 has two global variables, "bootmethod" and "efi_boot" which serve overlapping purposes? This suggests the situation already needs to be cleaned up.

I'll concede it might need two variables. My real concern is making this architecture independent as this resolves the situation with sys/dev/xen/efi/pvefi.c rather more elegantly than the approach of D31065 does.

In D31955#721084, @ehem_freebsd_m5p.com wrote:

In D31955#720853, @andrew wrote:

This is confusing two mostly independent concepts. One is the firmware type. On arm64 it is almost always UEFI, however it could also boot via the same ABI as a Linux kernel. The other is the hardware description method.

Yes, they are somewhat independent, but they are interrelated and strongly correlate.

Of the spots touched by this they all appear to ask one of two questions:

1. Use OpenFirmware device-tree or ACPI table?
2. Are UEFI runtime services available?

I'm under the impression using device-trees pretty well excludes full UEFI runtime services. The only case where these mix at all is U-Boot's UEFI support, which mostly appears to be good enough for bootloader use and only minimally implement the UEFI spec.
As such this seems to be a spectrum with device-trees on one end and full UEFI the other.

Why should the use of a device tree exclude the presence UEFI runtime services? The u-boot devs are cramming in a lot of new UEFI functionality these days.

In D31955#720857, @kib wrote:

On amd64 there is already 'efi_boot' var.

That actually concerns me. So AMD64 has two global variables, "bootmethod" and "efi_boot" which serve overlapping purposes? This suggests the situation already needs to be cleaned up.

I'll concede it might need two variables. My real concern is making this architecture independent as this resolves the situation with sys/dev/xen/efi/pvefi.c rather more elegantly than the approach of D31065 does.

I would suggest that you just implement the machdep.bootmethod sysctl and accompanying bootmethod variable on arm64. This idea has been brought up before, and there are scripts in the base system that check this sysctl already (e.g. bsdinstall), so it is established and not going away on amd64. The values could be {"UEFI", "LinuxABI", "XEN"}, or something along these lines.

If there are some UEFI runtime services that are not implemented on the Xen host, say get/set var, what would happen? It looks like the functions in pvefi.c would just propagate any errors and behave sensibly.

In D31955#721437, @mhorne wrote:

In D31955#721084, @ehem_freebsd_m5p.com wrote:

In D31955#720857, @kib wrote:

On amd64 there is already 'efi_boot' var.

That actually concerns me. So AMD64 has two global variables, "bootmethod" and "efi_boot" which serve overlapping purposes? This suggests the situation already needs to be cleaned up.

I'll concede it might need two variables. My real concern is making this architecture independent as this resolves the situation with sys/dev/xen/efi/pvefi.c rather more elegantly than the approach of D31065 does.

I would suggest that you just implement the machdep.bootmethod sysctl and accompanying bootmethod variable on arm64. This idea has been brought up before, and there are scripts in the base system that check this sysctl already (e.g. bsdinstall), so it is established and not going away on amd64. The values could be {"UEFI", "LinuxABI", "XEN"}, or something along these lines.

You should note the first version did something along this line. I preserved machdep.bootmethod as a sysctl derived from the value of firmware_type (see sys/x86/x86/cpu_machdep.c).

This seems an argument for making this even more architecture-independent. In which case where should set_bootmethod() and firmware_type be placed? New file in sys/kern?

I'm also unsure of where to get the appropriate values on ARM64. There are some reasonable spots to set firmware_type = FW_UEFI;, but not too many good spots for the other values.

Issues I'm wondering about with the current revision...

I'm wondering about native_parse_memmap(). I'm unsure it helps that much to avoid the redundancy and I'm wondering whether that should even be part of this patch.

I added bootmethod.c to files.arm64 and files.x86 since there is presently no support on other architectures. Does the core feel this should be put in files to encourage implementing similar functionality on the other architectures?

I'm wondering whether setting firmware_type should be added to native_parse_preload_data(). This would remove the if after that call and simply have a rule of it should always be set in the init_ops.parse_preload_data() call.

Now there is the problem ARM64 never had support for reporting the boot method, and I don't see good spots for detecting anything besides FW_UEFI.

I cannot tell you what and how arm platforms interpret statement "UEFI boot", but on amd64 the efi_boot == true means two things:

1. There is functional ESP partition, and management tools can look for it (used by installer)
2. There is no BIOS or CSM, so we must not touch miscellaneous low memory locations expecting that to affect reset/reboot/resume behavior of the platform.

This explicitly excludes e.g. answering the question "are runtime services present".

My goal is fixing sys/dev/xen/efi/pvefi.c to work with ARM64 kernel builds. Ideally this increases similarity between FreeBSD architectures. I was guessing some of the bits were depending on runtime services, but I could well be mistaken.

In other news in a test Xen VM on ARM64:

# sysctl machdep.bootmethod
machdep.bootmethod: UEFI
#

As the initial boot was a Tianocore/EDK2 image, that is the expected result. As such this does appear to give expected results on ARM64, for the one case which can be tested.

Presently I still need a recommendation for where to place the firmware_type declaration. @kib, you didn't like systm.h, but I'm unsure where else to place it.

Now an interesting bit with moving handling of the setting the sysctl value to subr_param.c is it looks reasonable to merge the setting of machdep.bootmethod with the setting of kern.vm_guest (both use similar constructs). I'll likely need a bit to get to implementing that though.

I'll concede several/most of the places checking for FW_UEFI should likely instead be checking for FW_BIOS (Xen not emulating that portion of the environment). Thing is, I believe fixing those is distinct from D31955.

My reason for creating D31955 is to make sys/dev/xen/efi/pvefi.c architecture-independent. For which the global x86 "bootmethod" and "efi_boot" variables need to be dealt with.

Any reviews? @kib wants reviews from ARM and/or Xen side of FreeBSD.

I suspect @kib is correct and those various tests should in fact be firmware_type == FW_BIOS, rather than firmware_type != FW_UEFI. I though think that should be distinct from D31955. Mainly the topic of D31955 is attempting to merge down the number of boot method variables and make the single remaining one architecture-independent (thus making the "pvefi.c" file architecture-independent), while leaving the logic unchanged.

This successfully detects UEFI on ARM, resulting in the sysctl value of machdep.bootmethod being "UEFI" in that type of boot. Otherwise it should get an empty string (if someone can point to an appropriate spot for selecting "LinuxABI" that can be added).