Add uefi::system module

[nicholasbishop]

This is similar to existing methods of SystemTable, but as freestanding functions that use the global system table pointer.

Splitting this out from #905, the system module provides a good starting point to get a sense of what uefi::{system,boot,runtime} will look like.

#893

Checklist

• Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
• Update the changelog (if necessary)

[phip1611]

Could you please add a usage in the test-runner as well?

[nicholasbishop]

Added tests (one of which required PartialEq on ConfigTableEntry, so I went ahead and added all the usual derives there).

[phip1611]

I'm sorry for postponing this further, but I'm unsure regarding the API. Here, we panic and return a NonNull. system_table_boot() and system_table_runtime() below return Options. Is this consistent? Or is it fine to have this difference between internal and public API?

[nicholasbishop]

No worries, happy to discuss API decisions. I think having both panicking and non-panicking variants is useful, depending on the circumstances. The function that gets the raw system table should essentially never fail -- the panic can only be reached if the table pointer is null, which means the user isn't using the entry macro, or has an unsafe call to set_system_table(null).

In the high-level API, we could (for example) change system::firmware_vendor to return a Result, with an error if the system table is null. But in practice, I think it's likely that users would simply unwrap the result anyway.

If we accept the above argument, that means will have a decent number of places where we want to panic internally if the system table isn't set. And for each of those places we want to use expect with a clear error message rather than unwrap, so if that panic is ever hit it's clear why. And since we don't want to write out .expect("global system table pointer is not set") in multiple places, we end up with this shared function.

I'll also note that I see the system_table_boot and system_table_runtime functions as transitional APIs. Once the system, boot, and runtime modules are in place, I want to mark SystemTable, BootServices, and RuntimeServices as deprecated, and then after a release or two we can remove those deprecated APIs entirely.

Now, with all that said, we might want a non-panicking variant in the future as well (e.g. for functions that already return a Result), so I've pushed a change to rename it to system_table_raw_panicking.

[phip1611]

But in practice, I think it's likely that users would simply unwrap the result anyway.

Agree

I'll also note that I see the system_table_boot and system_table_runtime functions as transitional API

got it

I want to mark SystemTable, BootServices, and RuntimeServices as deprecated, and then after a release or two we can remove those deprecated APIs entirely.

Is there already a todo anywhere for that?

---

Generally, I think panicking at a core function is perfectly fine. Especially, as this will never fail, if people properly use the #[entry] macro.

[nicholasbishop]

Is there already a todo anywhere for that?

Added a new summary comment here: #893 (comment)

[phip1611]

nit: In a follow-up: We might should add a [new api] "tag" for that in the changelog - this might ease consumers to understand what's going on

[nicholasbishop]

Agreed, I was thinking maybe just add a short paragraph between # uefi and ## Added .