Why is this sized based on 2 pages, rather than the more common single page size?

Well, that size was picked up from Windows implementation. I am open to a more appropriate size.

The windows backend picked 8k due to behavior of the windows system libraries (also see f411852). I don't think any of this reasoning applies to UEFI.

Why checking for the system-table here? It can be invalidated in a parallel call.

I am not sure what you mean. My intention was to check if the code panics before the system table is initialized in Rust (which can happen) or after it. It is not really meant to check if system table is valid for complete UEFI.

But why do you check for it? What is the rationale?

What is the reasoning here? You effectively fold every error to the default error by treating everything as EBADF, or what am I missing?

That's just the default unsupported implementation. I am not sure what EBADF corresponds to in UEFI. Feel free to comment about it.

is_ebadf() is used to suppress EBADF errors, and avoid panicking on println!() if the output-stream does not exist. Your current implementation for UEFI causes ALL errors for output-streams to be suppressed, because you unconditionally return true.

There is no equivalent to EBADF on UEFI, since you operate on devices directly, rather than on virtualized FDs. However, you likely want to suppress EFI_UNSUPPORTED, since this is triggered when text-mode is currently disabled. But is_ebadf() seems like the wrong place to do this.

Maybe use an explicit type-annotation here? You rely on the size of the type by hard-coding 2, yet no explicit type is used.

Yeah, I guess it can be changed to size_of::<u16>(). I started with the windows implementation so it is from that.

I am not talking about the sizeof_of, I am talking about the type-annotation of utf16 (let mut utf16: u16 = ...).

The - 1 should be dropped. floor_char_boundary() will correctly use the entire string if the index is beyond the last character.

So the -1 is so that the string is NULL terminated.

Fair enough. Though, I think this is really not obvious from this code. And simple_text_output() takes a slice but silently expects it to be NULL-terminated. I think this is very likely to lead to problems. I would prefer at least an assertion in simple_text_output() to catch such errors in the future.

Why not error out if the system-table is not available?

So the current implementation has a default behaviour to panic if system table (and brothers) is not available. The exceptions are when the function might be called before sys::init.

Yes, I know. I want to understand what you want the behavior to be when it is called before sys::init? You check for validity in the panic-path, but not here, which seems inconsistent. I am simply trying to understand what you want it to behave like?

Can you explain why you strip zeros?

It's not stripping zeros, scan_code not 0 indicates a non-printable character. See Section 12.3.3 in UEFI Spec

unicode_char == 0x00 represents control characters. The scan-code is always set, yet you check the scan-code rather than unicode_char.

If I am not mistaken, your current code suppresses all input.

The ReadKeyStroke() function reads the next keystroke from the input device. If there is no pending keystroke the
function returns EFI_NOT_READY. If there is a pending keystroke, then ScanCode is the EFI scan code defined in
EFI Scan Codes for EFI_SIMPLE_TEXT_INPUT_PROTOCOL . The UnicodeChar is the actual printable character or
is zero if the key does not represent a printable character (control key, function key, etc.)

I think it is overly optimistic to rely on firmware never returning invalid data. I would prefer to forward errors, but I will not insist.

Since we are reading a printable key press (due to the above check), I would hope it would be valid. But since it is possible to do file piping for input, I guess it might be good to return an error.

You drop data if the buffer is short. This is really unfortunate. I think the better alternative would be to have an internal buffer in struct Stdin where you keep the last char and keep trying to return it to the caller until they provide a suitable buffer. This does busy-loop if the caller never provides a suitable buffer, but at least you do not drop data silently.

Alternatively, return exactly the requested amount of bytes, and return the remaining ones in following calls. There is no reason why the caller would expect every read-call to split exactly at UTF8-boundaries, or is there?

Yes, it seems like a reasonable strategy. I think an internal buffer of a single character (u16) should be enough.

Any particular reason you do not loop on NOT_READY? Can we rely on wait_stdin to never report spurious wakeups?

Well, I am not sure busy waiting would be the best idea. In all the examples of reading I have seen online, everyone seems to rely on the con_in->wait_for_key event.
The spec also states the following: "Event to use with EFI_BOOT_SERVICES.WaitForEvent() to wait for a key to be available." So it should be the best solution.

I am not suggesting busy-waiting. I am saying to loop on NOT_READY. So after wait_stdin(), you still check for NOT_READY and wait again if it keeps getting reported.