Thanks for the PR! A few comments:

• So far we relied on all function-pointers to be provided behind a raw pointer. That is, SystemTable et al are only provided as *mut SystemTable, and thus you need an unsafe block to fetch the function-pointer provided in it. So at least I don't think there is any safety issue in the basic UEFI setup.
• Adding unsafe means you cannot implement a safe version of such a function, which is a bit sad. I think that is the reason we omitted it initially. However, you are right that any function that dereferences a raw pointer cannot reasonably be implemented without an unsafe annotation. And even most by-value types are just wrapped raw pointers (like efi::Event).
• If we go for this, it is a major API break and downstream effects need to be considered.

I am generally not opposed to this. Major users are Patina (@makubacki @joschock) and rust::std (@Ayush1325), maybe they want to chime in?

Btw., did you auto-generate this? If yes, can you append the script? This is usually easier to review and audit, then the raw diff.

I first really hit upon this issue when using BootCreateEvent, which accepts a callback as its third parameter. Currently that callback type, EventNotify, is a safe fn—however writing such a callback function in Rust without it being marked unsafe feels very wrong: the implementor relies upon callers upholding safety preconditions (eg that the context, if provided, is valid). And, of course, one cannot provide an unsafe fn where a safe one is expected.

Admittedly, that issue only applies to callbacks; and one could argue that because "it is known" both that the UEFI platform will uphold the safety preconditions and that the callback is not called elsewhere, the implementor can author the function without it having an unsafe signature.

Moreover, whilst I agree that accessing the UEFI function-pointers does require some original use of unsafe, pushing all the safety requirements of every subsequent function call onto that first use of unsafe essentially again requires one to reason globally, but now about almost one's entire application. IMHO Rust's traditional ethos is to keep safety reasoning as localised as possible.

I'm not too sure that I understand your second point (that "you cannot implement a safe version of such a function")? An unsafe fn type can hold a safe fn (provided the signatures are otherwise equivalent), so even if EventNotify is made unsafe, implementors can still provide safe implementations (eg if their implementation does not impose safety conditions upon their callers). Perhaps you mean that calling UEFI functions will now require unsafe blocks? That is indeed true, but surely also the point.

As regards auto-generation: no, I'm afraid I just used a vim macro as I manually reviewed each occurrence of eficall within the source. The following are the only invocations of the eficall! macro that I didn't apply unsafe to:

• protocols::device_path_utilities::ProtocolCreateDeviceNode
• protocols::shell::BatchIsActive
• protocols::shell::IsRootShell
• protocols::shell::EnablePageBreak
• protocols::shell::DisablePageBreak
• protocols::shell::GetPageBreak
• protocols::timestamp::ProtocolGetTimestamp
• system::BootRaiseTpl
• system::BootRestoreTpl
• system::BootStall

@dvdhrm, thank you for reaching out to us. While this will result in a lot of code churn, we've discussed this within the Patina project and we're in favor of this change. We believe this better makes use of unsafe to help consistently structure the codebase alongside safe wrappers and other code.

@dvdhrm Thanks for reaching out. I am in favor of the change as well.