Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33737 )
Change subject: device/oprom/realmode: Add vbe return status support as per VBE spec 3.0 ......................................................................
Patch Set 14:
(3 comments)
https://review.coreboot.org/c/coreboot/+/33737/14/src/device/oprom/realmode/... File src/device/oprom/realmode/x86.c:
https://review.coreboot.org/c/coreboot/+/33737/14/src/device/oprom/realmode/... PS14, Line 255: default:
I'm not sure about the `default`, if VBE ever gets extended or some […]
It's unlikely that anybody will ever release a new revision of VBE. PCBIOS is dead. As for non-standard codes (or retro-computing fans who might extend VBE), let's go there when we see them - maybe they'd give us a license for Scitech Display Doctor, too.
https://review.coreboot.org/c/coreboot/+/33737/14/src/device/oprom/realmode/... PS14, Line 271: X86_EAX
Why use the register file here? wouldn't a local variable be enough? or […]
I found that rather neat in terms of "that happened, it had an impact on realmode, so make the realmode impact consistent". Overall it won't matter much though, since users will mostly set their registers the way they need them before a call.
So in general it's a pattern that might be interesting to promote in this subsystem, but practically speaking, this subsystem is at the end of its lifetime, and it's hard for me to care about such matters of taste in that case.
https://review.coreboot.org/c/coreboot/+/33737/14/src/device/oprom/realmode/... PS14, Line 274: die("\nError: In %s function\n", __func__);
I don't think we should die(), modesetting errors don't seem generally fatal?
So calling into unknown binaries is okay (VGABIOS hanging was a classic for me, especially pre-YABEL), but complaining loudly when they return an error is not?
It's unlikely that these functions will only sometimes fail (for a given state or input). Under that assumption die() helps with debugging before shipping releases. Again it's a matter of taste (and I might have done it differently, too).