Would it make sense to add a fn unmap(self) -> Result method, so that callers can choose to handle errors from unmapping? The Drop trait doesn't really allow that. I think the drop implementation should probably not panic on errors, and that we should remove the unreachable; UEFI implementations sometimes return additional errors beyond what the spec includes.

Ditto for the PciPage implementation.

True, I initially put unreachable macro there to indicate that "errors that may happen according to spec are all handled and no other error can occur"
But yeah second thought we can never be sure

Tho, what else should it do when an error occurs? Should I make it undroppable(assuming it's possible) so that users must use functions like unmap?

There's no way to make something undroppable in Rust, unfortunately. There are two options:

• A 'drop bomb', i.e. panic unconditionally in drop if unmap wasn't called. The documentation should strongly remind the user that unmap must be called.
• Handle errors in drop by ignoring them (or log them and otherwise ignore). The documentation can encourage users to call unmap for better error handling, but it's not required. This is what File does, for example; it suggests calling sync_all before drop if you want to handle errors.

In cases where errors are unlikely (which I think is the case here), I prefer the 2nd option; log errors in drop but otherwise ignore them.