Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Veto for Insyde Serial Terminal Driver. #1193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hetii
Copy link

@hetii hetii commented Apr 16, 2024

This pull request is to solve uart issue described in the issue #1189

@mcb30
Copy link
Member

mcb30 commented Apr 16, 2024

I think we need to have the veto take effect only if CONSOLE_SERIAL is enabled, in the same way that efi_veto_hp_xhci() will veto the HP xHCI driver only if we are expecting to drive the xHCI hardware ourselves.

This needs to be done in a way that doesn't involve putting the relevant code inside #ifdef CONSOLE_SERIAL because that's a pattern I strongly avoid in iPXE (see e.g. https://dox.ipxe.org/ifdef_harmful.html). It also can't use a symbol reference that would cause the serial console code to be dragged in unconditionally. Possible solutions are to use a sideband lookup mechanism (as for xHCI, though I don't think there's anything directly equivalent we could use for serial), to use a weak symbol (see e.g. vlan_tci() in netdevice.c), or to use a limited #ifdef block that merely assigns a constant value (see e.g. shim_cmd.c).

Some thought needs to be given as to whether the veto should depend upon the presence of serial.o or uart.o: the former indicates that we are expecting to control the serial console directly, the latter indicates that we are expecting to control a serial port directly for either console or some other purpose (e.g. a gdb connection).

With the above solved, I wonder if it would be sensible to drop the restriction on the vendor being Insyde, and have the veto take effect for anything that we can identify as a serial port driver if iPXE is expecting to drive the serial port directly itself? In the default configuration (no CONSOLE_SERIAL) we would not veto anything. Enabling CONSOLE_SERIAL in an EFI build would then mean "take ownership of and use the serial port" rather than the current "use the serial port and hope that nothing else is already using it".

@hetii hetii force-pushed the veto-insyde-serial-terminal-driver branch from dffdd70 to 5ab4910 Compare April 16, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants