Hey folks, this is the 2nd iteration of the patchset adding a simple mechanism to notify the UEFI firmware about a panic event in the kernel. V1 here: https://lore.kernel.org/lkml/20220520195028.1347426-1-gpiccoli@xxxxxxxxxx/ First thing, the differences in this V2: - Ardb response in V1 mentioned a refactor aimed for v5.20 that removes an obsolete/confusing way of setting EFI variables - this led to a weird condition, deleted variables stayed in sysfs after deletion. Well, I've refactored the code based on efi/next, so I'm using the recommended API now - thanks a bunch for the advice Ardb! - I've changed NULL-terminating char in patch 1 to the format I've seen in Ardb's code, L'\0'. - Patch 2 is new, it's somewhat a fix for a patch only in efi/next, part of the efivar refactor. In the V1 review, it was mentioned we could maybe use efi-pstore as a way to signal the firmware about a panic event - in the end, the efi-pstore mechanism can collect a dmesg, so it's even richer in the information level. But I disagree that it is the way to go, for 3 main reasons: a) efi-pstore could be impossible to use, if the users are already using another pstore backend (like ramoops), which is _exactly_ our case! Of course, we could rework pstore and allow 2 backends, quite a bit of work, but...see next points! b) Even if (a) is a not an issue, we have another one, even more important: signaling the firmware about a panic is *different* than collecting a bunch of data, a full dmesg even. This could be considered a security issue for some users; also, the dmesg collected consumes a bunch more memory in the (potentially scarce) UEFI available memory. Although related, the goal of pstore is orthogonal to our mechanism here: users rely on pstore to collect data, our proposal is a simple infrastructure to just let the firmware know about a panic. Our kernel module also shows a message and automatically clears the UEFI variable, so it tracks a single panic, whereas efi-pstore logs are kept by default, in order to provide data to users. c) Finally, it's faster and less "invasive"/risky to just write a byte in a variable on a panic event than having a ksmg dumper collecting the full dmesg and writing it to the UEFI memory; again, some users wish to have the logs, but not all of them. With all of that said, I think this module makes sense, it's a very simple solution that opens doors to firmware panic handling approaches, like in our proposed case (a different splash screen on panic). Finally, the variable name (PanicWarn) and value (0xFF by default, can be changed by a module parameter) are just my personal choices but I'm open to suggestions, not strongly attached to them heh Thanks again for the reviews/suggestions! Cheers, Guilherme Guilherme G. Piccoli (3): efi: Add a generic helper to convert strings to unicode efi: efibc: Guard against allocation failure efi-panic: Introduce the UEFI panic notification mechanism drivers/firmware/efi/Kconfig | 10 ++++ drivers/firmware/efi/Makefile | 1 + drivers/firmware/efi/efi-panic.c | 89 +++++++++++++++++++++++++++++++ drivers/firmware/efi/efi-pstore.c | 4 +- drivers/firmware/efi/efibc.c | 11 ++-- include/linux/efi.h | 18 +++++++ 6 files changed, 125 insertions(+), 8 deletions(-) create mode 100644 drivers/firmware/efi/efi-panic.c -- 2.37.1
