std.os.uefi.tables: ziggify boot and runtime services #23441

dotcarmen · 2025-04-02T17:18:14Z

this PR updates BootServices and RuntimeServices to an idiomatic interface similar to the pattern used in std.os.uefi.protocol. Part of my crusade towards making std.os.uefi more idiomatic

dotcarmen · 2025-04-02T17:18:59Z

@linusg

linusg · 2025-04-02T17:21:40Z

another-one.png

It is appreciated! :^)

linusg

Not a thorough review, LGTM at a high level

lib/std/os/uefi.zig

lib/std/os/uefi/tables/boot_services.zig

dotcarmen · 2025-04-02T17:27:55Z

lib/std/os/uefi/tables/boot_services.zig


    /// Returns the current memory map.
-    getMemoryMap: *const fn (mmap_size: *usize, mmap: ?[*]MemoryDescriptor, map_key: *usize, descriptor_size: *usize, descriptor_version: *u32) callconv(cc) Status,
+    _getMemoryMap: *const fn (mmap_size: *usize, mmap: [*]MemoryDescriptor, map_key: *MemoryMapKey, descriptor_size: *usize, descriptor_version: *u32) callconv(cc) Status,


i figured it's safer to obscure map_key values since as far as i can tell it's meant to be assigned by the system (only retrievable via getMemoryMap)

lib/std/os/uefi/tables/boot_services.zig

dotcarmen · 2025-04-02T17:46:22Z

lib/std/os/uefi/tables/boot_services.zig

+        interfaces: anytype,
+    ) InstallProtocolInterfacesError!Handle {
+        var hdl: ?Handle = handle;
+        const args_tuple = protocolInterfaces(&hdl, interfaces);


i couldn't find an easier way to manage dynamically constructing a tuple than this helper :/

dotcarmen · 2025-04-02T17:47:35Z

lib/std/os/uefi/tables/boot_services.zig

 };
+
+fn protocolInterfaces(


this function is kinda gross :/ suggestions are welcome

lib/std/os/uefi/tables/boot_services.zig

truemedian

A few minor nitpicks, but since we're overhauling boot services it's time we remove [*]MemoryDescriptor and []MemoryDescriptor from std.os.uefi, they are never valid because the size of the zig struct is almost never the size of the descriptor the firmware returns.

lib/std/os/uefi/tables/boot_services.zig

lib/std/os/uefi/tables.zig

dotcarmen · 2025-04-03T15:09:03Z

lib/std/os/uefi/tables/boot_services.zig

-    raiseTpl: *const fn (new_tpl: usize) callconv(cc) usize,
+    raiseTpl: *const fn (new_tpl: TaskPriorityLevel) callconv(cc) TaskPriorityLevel,

    /// Restores a task's priority level to its previous value.
-    restoreTpl: *const fn (old_tpl: usize) callconv(cc) void,
+    restoreTpl: *const fn (old_tpl: TaskPriorityLevel) callconv(cc) void,


doesn't make sense to define wrappers for these functions imo

lib/std/os/uefi/tables/boot_services.zig

dotcarmen · 2025-04-03T15:15:33Z

lib/std/os/uefi/tables/boot_services.zig


-    /// Opens a protocol with a structure as the loaded image for a UEFI application
-    pub fn openProtocolSt(self: *BootServices, comptime protocol: type, handle: Handle) !*protocol {


this function is now basically the handleProtocol function

lib/std/os/uefi/tables.zig

lib/std/os/uefi/tables/runtime_services.zig

lib/std/os/uefi/tables/boot_services.zig

lib/std/Thread.zig

lib/std/os/uefi/pool_allocator.zig

dotcarmen · 2025-04-04T16:33:13Z

lib/std/debug.zig

@@ -639,7 +639,8 @@ pub fn defaultPanic(
                // ExitData buffer must be allocated using boot_services.allocatePool (spec: page 220)
                const exit_data: []u16 = uefi.raw_pool_allocator.alloc(u16, exit_msg.len + 1) catch @trap();
                @memcpy(exit_data, exit_msg[0..exit_data.len]); // Includes null terminator.
-                _ = bs.exit(uefi.handle, .aborted, exit_data.len, exit_data.ptr);


afaict this was a bug 😬 exit_data.len for number of u16s but the spec says it's the number of bytes

dotcarmen · 2025-04-04T16:33:55Z

i feel like this is ready for a final review :) i haven't tried it with my own project yet though, i'll tackle that later

dotcarmen · 2025-04-04T18:15:28Z

lib/std/os/uefi/tables/boot_services.zig

@@ -134,7 +134,7 @@ pub const BootServices = extern struct {
    _setWatchdogTimer: *const fn (timeout: usize, watchdog_code: u64, data_size: usize, watchdog_data: ?[*]const u16) callconv(cc) Status,

    /// Connects one or more drives to a controller.
-    _connectController: *const fn (controller_handle: Handle, driver_image_handle: ?[*:null]Handle, remaining_device_path: ?*const DevicePathProtocol, recursive: bool) callconv(cc) Status,


really feels like [*:null]*anyopaque should work :(

lib/std/os/uefi/tables/runtime_services.zig

lib/std/os/uefi.zig

lib/std/os/uefi/pool_allocator.zig

lib/std/os/uefi/tables.zig

lib/std/os/uefi/tables/runtime_services.zig

lib/std/os/uefi.zig

lib/std/os/uefi/tables/runtime_services.zig

lib/std/os/uefi/tables/boot_services.zig

lib/std/os/uefi/tables/runtime_services.zig

lib/std/os/uefi/tables/boot_services.zig

dotcarmen · 2025-04-10T17:03:27Z

haven't forgotten or abandoned this, I'm dealing with immigration stuffs :) I'll pick it up again soon

dotcarmen

ok i may have gone a little overboard with 7372d65 so i partly reverted it in 04e3728 to keep the number of files in the diff small :) those align(8)s aren't an error or anything, they're just unnecessary now :)

otherwise the rest of the PR comments have been addressed!

dotcarmen · 2025-04-13T19:04:02Z

lib/std/os/uefi.zig

 /// A handle to an event structure.
 pub const Event = *opaque {};

+pub const EventRegistration = opaque {};


I decided to not keep this as *const opaque{} since it might be nice to have the namespace eventually? idk

I actually feel like it'd be handy to do that for Event eventually 🙃

The standard describes an event registration as a VOID*, and the standard provides absolutely no methods to operate on the registration value other than passing around the VOID*, so I say it makes no sense to change this from a *opaque{}.

If for some reason someone finds a good reason to make this a namespace that would be a better time to make this change.

lib/std/os/uefi/tables/boot_services.zig

Co-authored-by: linusg <mail@linusgroh.de>

truemedian · 2025-04-17T19:35:24Z

lib/std/os/uefi.zig

 /// A handle to an event structure.
 pub const Event = *opaque {};

+pub const EventRegistration = opaque {};


The standard describes an event registration as a VOID*, and the standard provides absolutely no methods to operate on the registration value other than passing around the VOID*, so I say it makes no sense to change this from a *opaque{}.

If for some reason someone finds a good reason to make this a namespace that would be a better time to make this change.

lib/std/os/uefi/tables.zig

lib/std/os/uefi/tables/runtime_services.zig

dotcarmen

decided to include a pattern that i've been experimenting with in another PR i'm working on for std.elf, wdyt?

dotcarmen · 2025-04-18T15:08:09Z

lib/std/os/uefi/tables.zig

+    pub const InvalidValue = math.IntFittingRange(
+        @intFromEnum(MemoryType.invalid_start),
+        @intFromEnum(MemoryType.invalid_end),
+    );
+    pub const OemValue = math.IntFittingRange(
+        @intFromEnum(MemoryType.oem_start),
+        @intFromEnum(MemoryType.oem_end),
+    );
+    pub const VendorValue = math.IntFittingRange(
+        @intFromEnum(MemoryType.vendor_start),
+        @intFromEnum(MemoryType.vendor_end),
+    );


define IntFittingRange for the relevant value range

oh wait the start should be 0 😅 and the end should be _start - _end 😅

_end - _start *

dotcarmen · 2025-04-18T15:08:52Z

lib/std/os/uefi/tables.zig

+    pub fn invalid(value: InvalidValue) MemoryType {
+        const invalid_start = @intFromEnum(MemoryType.invalid_start);
+        return @enumFromInt(invalid_start + value);
+    }


function for constructing such values ie pub const my_invalid: MemoryType = .invalid(0x2);

Why are we constructing invalid memory types?

dotcarmen · 2025-04-18T15:09:25Z

lib/std/os/uefi/tables.zig

+    pub fn getInvalid(memtype: MemoryType) ?InvalidValue {
+        const as_int = @intFromEnum(memtype);
+        const invalid_start = @intFromEnum(MemoryType.invalid_start);
+        if (as_int < invalid_start) return null;
+        if (as_int > @intFromEnum(MemoryType.invalid_end)) return null;
+        return @truncate(as_int - invalid_start);
+    }


function for retrieving the value relative to the range's start

kouosi · 2025-04-18T16:28:42Z

IMO, it is also a good idea to use _allocatePage in pool_allocator if alignment is 4096, as _allocatePool is slow.

truemedian · 2025-04-18T16:34:32Z

Not that that's a bad idea for the special case of exactly 4K alignment, I definitely would need a source to back up that allocatePool is significantly slower than allocatePages on at least a few firmware UEFI implementations

kouosi · 2025-04-18T17:04:15Z

Not that that's a bad idea for the special case of exactly 4K alignment, I definitely would need a source to back up that allocatePool is significantly slower than allocatePages on at least a few firmware UEFI implementations

I don't have an exact source to back this up, but testing on some hardware I have available I found that allocatePage is significantly slower than allocatePool.

dotcarmen · 2025-04-18T19:37:39Z

IMO, it is also a good idea to use _allocatePage in pool_allocator if alignment is 4096, as _allocatePool is slow.

tbh i think that's a better change for #22818 which is doing a lot more work to integrate uefi's allocator api into zig's

RossComputerGuy

Generally looks good to me.

lib/std/os/uefi/tables.zig

RossComputerGuy · 2025-04-18T22:49:57Z

Not that that's a bad idea for the special case of exactly 4K alignment

Yeah, it's good that the spec mentions that only 4k page sizes are used.

lib/std/os/uefi.zig

lib/std/os/uefi/tables/runtime_services.zig

lib/std/os/uefi/tables/boot_services.zig

This backs out commit cdd9bd6.

dotcarmen · 2025-04-29T12:33:23Z

another PR i'm working on for std.elf

now #23600 btw :)

dotcarmen · 2025-05-10T20:58:15Z

friendly ping :) @linusg

RossComputerGuy

LGTM

linusg · 2025-05-10T21:51:08Z

@kouosi / @truemedian any final thoughts?

truemedian

The memory type abstracting the separate invalid/vendor/oem fields into their own thing kind of confuses me; invalid in particular because you should never have any reason to construct or know which invalid field you have outside of "it's invalid".

truemedian · 2025-05-10T22:15:54Z

lib/std/os/uefi/tables.zig

+    pub fn invalid(value: InvalidValue) MemoryType {
+        const invalid_start = @intFromEnum(MemoryType.invalid_start);
+        return @enumFromInt(invalid_start + value);
+    }


Why are we constructing invalid memory types?

dotcarmen · 2025-05-10T22:26:03Z

i can remove fn invalid, but the others make sense imo - users may want to reference a vendor's memory type that isn't defined here in std, or define their own, etc.

i just wrote fn invalid to satisfy my own symmetry itch :)

truemedian · 2025-05-10T22:27:44Z

The concern there is that the vendor or oem's memory type is most certainly not going to be an offset from the offset of vendor/oem types. It's going to just be a single hex value that you could @enumFromInt().

(also it should be noted that a wide variety of UEFI firmware derived from Tianocore struggles to handle vendor memory types)

dotcarmen · 2025-05-11T10:14:46Z

The concern there is that the vendor or oem's memory type is most certainly not going to be an offset from the offset of vendor/oem types. It's going to just be a single hex value that you could @enumFromInt().

A decent number of the constants I refactored were using offsets from oem/vendor types. While that alone isn't a good argument, it becomes a lot more apparent to the user what exactly they're referencing, which I think is an easy win for readability.

Of course enumFromInt will still exist for people who are using a cImport or plain don't want to.

(also it should be noted that a wide variety of UEFI firmware derived from Tianocore struggles to handle vendor memory types)

Can you expand on this? Are you saying that UEFI bootloaders developed in a Tianocore environment don't handle real-world scenarios?

truemedian · 2025-05-11T17:03:31Z

Can you expand on this? Are you saying that UEFI bootloaders developed in a Tianocore environment don't handle real-world scenarios?

It's a bug in at least the QEMU firmware and a few other virtual machine platforms and people have encountered it in the wild on hardware. Essentially the firmware reads an array out of bounds when it encounters a memory type it doesn't know, including the user-defined range.

rust-osdev/uefi-rs#1375
https://stackoverflow.com/questions/67425959/can-bootloaders-use-custom-uefi-memorytypes
https://wiki.osdev.org/UEFI#My_bootloader_hangs_if_I_use_user_defined_EFI_MEMORY_TYPE_values

dotcarmen · 2025-05-11T21:34:22Z

i think the conclusions in those links are reasonable - that bug is platform-specific and independent of the design of this api. if you use a non-buggy UEFI firmware, then it's reasonable to expect to be able to use custom memory types, and these functions help with that (besides fn invalid)

lib/std/os/uefi/tables/boot_services.zig

lib/std/os/uefi/tables/runtime_services.zig

dotcarmen force-pushed the ziggify-uefi-boot-runtime-services branch from ee41a75 to e4487d5 Compare April 2, 2025 17:22

linusg reviewed Apr 2, 2025

View reviewed changes

linusg mentioned this pull request Apr 2, 2025

std: rework uefi allocators #22818

Open

dotcarmen commented Apr 2, 2025

View reviewed changes

truemedian reviewed Apr 2, 2025

View reviewed changes

dotcarmen force-pushed the ziggify-uefi-boot-runtime-services branch from e4487d5 to 5ad806e Compare April 2, 2025 18:35

dotcarmen mentioned this pull request Apr 2, 2025

std.os.uefi.protocol.File: fix some typed definitions #23443

Merged

dotcarmen commented Apr 3, 2025

View reviewed changes

truemedian reviewed Apr 3, 2025

View reviewed changes

lib/std/os/uefi/tables.zig Outdated Show resolved Hide resolved

lib/std/os/uefi/tables.zig Outdated Show resolved Hide resolved

lib/std/os/uefi/tables/runtime_services.zig Outdated Show resolved Hide resolved

lib/std/os/uefi/tables/boot_services.zig Outdated Show resolved Hide resolved

dotcarmen force-pushed the ziggify-uefi-boot-runtime-services branch from 50823da to 0733531 Compare April 4, 2025 14:02

dotcarmen commented Apr 4, 2025

View reviewed changes

lib/std/os/uefi/tables/runtime_services.zig Show resolved Hide resolved

truemedian reviewed Apr 4, 2025

View reviewed changes

truemedian reviewed Apr 5, 2025

View reviewed changes

lib/std/os/uefi.zig Outdated Show resolved Hide resolved

lib/std/os/uefi/tables/runtime_services.zig Outdated Show resolved Hide resolved

lib/std/os/uefi/tables/boot_services.zig Outdated Show resolved Hide resolved

lib/std/os/uefi/tables/runtime_services.zig Show resolved Hide resolved

bnuuydev reviewed Apr 6, 2025

View reviewed changes

lib/std/os/uefi/tables/boot_services.zig Outdated Show resolved Hide resolved

bnuuydev reviewed Apr 6, 2025

View reviewed changes

lib/std/os/uefi/tables/boot_services.zig Outdated Show resolved Hide resolved

dotcarmen commented Apr 13, 2025

View reviewed changes

bnuuydev reviewed Apr 13, 2025

View reviewed changes

lib/std/os/uefi/tables/boot_services.zig Outdated Show resolved Hide resolved

dotcarmen and others added 7 commits April 13, 2025 23:44

std.os.uefi.tables: ziggify boot and runtime services

29d8aba

avoid T{} syntax

72575cd

Co-authored-by: linusg <mail@linusgroh.de>

misc fixes

7349eb6

work

e360e3a

self-review quickfixes

4193b88

dont make MemoryMapSlice generic

73231e2

more review fixes, work

8eee175

truemedian reviewed Apr 17, 2025

View reviewed changes

dotcarmen added 2 commits April 18, 2025 17:04

MapInfo consistency, MemoryType update per review

fb0d6f0

turn EventRegistration back into a pointer

c5006d6

dotcarmen commented Apr 18, 2025

View reviewed changes

forgot to finish updating MemoryType methods

423eff7

fix IntFittingRange calls

66e26a4

RossComputerGuy reviewed Apr 18, 2025

View reviewed changes

lib/std/os/uefi/tables.zig Show resolved Hide resolved

set uefi.Page nat alignment

cdd9bd6

RossComputerGuy reviewed Apr 20, 2025

View reviewed changes

lib/std/os/uefi.zig Outdated Show resolved Hide resolved

lib/std/os/uefi/tables/runtime_services.zig Outdated Show resolved Hide resolved

lib/std/os/uefi/tables/boot_services.zig Show resolved Hide resolved

dotcarmen added 3 commits April 21, 2025 20:47

Back out "set uefi.Page nat alignment"

0b6cacd

This backs out commit cdd9bd6.

get rid of some error.NotFound-s

d86621d

fix .exit call in panic

fae84f1

RossComputerGuy approved these changes May 10, 2025

View reviewed changes

truemedian reviewed May 10, 2025

View reviewed changes

kouosi reviewed May 12, 2025

View reviewed changes

dotcarmen added 2 commits May 12, 2025 18:23

review comments, add format method

4cb30ca

fix resetSystem data alignment

5480139


		/// Opens a protocol with a structure as the loaded image for a UEFI application
		pub fn openProtocolSt(self: BootServices, comptime protocol: type, handle: Handle) !protocol {

Uh oh!

std.os.uefi.tables: ziggify boot and runtime services #23441

Are you sure you want to change the base?

std.os.uefi.tables: ziggify boot and runtime services #23441

Uh oh!

Conversation

dotcarmen commented Apr 2, 2025

Uh oh!

dotcarmen commented Apr 2, 2025

Uh oh!

linusg commented Apr 2, 2025

Uh oh!

linusg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

truemedian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dotcarmen commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dotcarmen commented Apr 4, 2025 •

edited

Loading

dotcarmen Apr 18, 2025 •

edited

Loading

kouosi commented Apr 18, 2025 •

edited

Loading