Handle Virtual Consoles and Framebuffer drivers when enabling/disabling VGA #4648
Conversation
rene
force-pushed
the
handle-fb-vt-vga
branch
2 times, most recently
from
b2fc482 to
5ef8278
Compare
rene
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4648 +/- ##
=======================================
Coverage 21.07% 21.07%
=======================================
Files 8 8
Lines 1167 1167
=======================================
Hits 246 246
Misses 861 861
Partials 60 60 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Looking to @rucoder to review the details.
Even though this doesn't promise to make iGPU passthrough work on x86, what behavior should we see when debug.enable.vga is enabled and disabled on x86 with an iGPU? And have we tested that we see that behavior?
Good question @eriknordmark , yes, I've tested on a real x86 device with an iGPU. Depending on the device, the behavior can be one of the following:
Why I'm not fully covering iGPU passthrough on this PR? Because although iGPU passthrough might work with Seabios, there is one potential issue observed on this real device: trying to passthrough iGPU with UEFI OVMF BIOS can freeze the device, so in order to fully support iGPU passthrough, we also need to do some work on our OVMF BIOS... No other issues were observed if we don't passthrough the iGPU, so I think this PR is safe... |
| checkIoBundleAll(ctx) | ||
|
|
||
| // Nothing to do if VGA is already enabled | ||
| if vgaSwitch { |
There was a problem hiding this comment.
What is the correlation between vgaSwitch and gcp.GlobalValueBool(types.VgaAccess) != ctx.vgaAccess ?
IMO introducing this global variable makes it a lot harder to understand when what is happening.
There was a problem hiding this comment.
vgaSwitch it's just a variable to indicate whether we really need to trigger the switch. For instance, VGA will be always enabled when the device boots, if debug.enable.vga is true, no actions needs to be performed..... or if you set debug.enable.vga to false but the video is already disabled, no needs to perform the actions again...
| vt, err := unix.IoctlGetInt(fd, VT_OPENQRY) | ||
| if err != nil { | ||
| if err := unix.Close(fd); err != nil { | ||
| log.Errorf("Cannot close file descriptor: %v", err) |
There was a problem hiding this comment.
also here you log if closing the filedescriptor fails, but you don't log if ioctl fails, although IMO that is the more important information
There was a problem hiding this comment.
Yes, that's most important, for sure... that's why is the ioctl's error that is returned (in this case), and not the close(fd) error. The returned error will be logged by the caller... I just log close(fd) error here because I will not return it, TBH that's not super useful to log this error, it could be ignored, besides be very unlike to happen, the possible errors are: non-valid fd, syscall was interrupted or I/O error occurred... at the point we want to close the fd we shouldn't care too much anymore... but I'm logging it anyways...
| // Open default console file device | ||
| fd, err := unix.Open("/dev/tty0", unix.O_RDWR, 0) |
There was a problem hiding this comment.
| // Open default console file device | |
| fd, err := unix.Open("/dev/tty0", unix.O_RDWR, 0) | |
| consoleFileDevice, err := unix.Open("/dev/tty0", unix.O_RDWR, 0) |
Add function description to make revive plugin pass. Signed-off-by: Renê de Souza Pinto <[email protected]>
The log message "updateVgaAccess(%t)" was wrongly showing the value of ctx.usbAccess. Signed-off-by: Renê de Souza Pinto <[email protected]>
This commit adds a new file with helper functions to handle Virtual Terminals (VTs) and Framebuffer drivers. They will be required to detach/attach VTs and Framebuffers when VGA access is disabled/enabled. Signed-off-by: Renê de Souza Pinto <[email protected]>
When VGA is enabled/disabled, the PCIe VGA device is assigned/unassigned to/from the host through the vfio-pci driver mechanism. However, this might not be enough for integrated graphics adapters. This commit adds additional procedures that detach all framebuffer drivers and Virtual Terminals when VGA access is disabled and attach them back when VGA is enabled. Signed-off-by: Renê de Souza Pinto <[email protected]>
Add a short notice that on some devices a reboot might be required in order to get VGA console back after access is enabled. Signed-off-by: Renê de Souza Pinto <[email protected]>
Fix typo: controler -> controller Signed-off-by: Renê de Souza Pinto <[email protected]>
|
Updates in this PR:
|
Description
When VGA is enabled/disabled, the PCIe VGA device is assigned/unassigned to/from the host through the vfio-pci driver mechanism. However, this might not be enough for integrated graphics adapters (iGPUs). This PR adds additional procedures that detach all framebuffer drivers and Virtual Terminals when VGA access is disabled and attach them back when VGA is enabled.
Non-PCIe graphics cards
On devices with integrated graphics card, such as those present on many arm64 SoCs, the vfio-pci mechanism cannot be applied. Thus, on these devices video remains active even though
debug.enable.vgais disabled. The changes introduced by this PR will also address these devices, so the expected behavior will now be applied.iGPU passthrough
The procedure introduced by this PR might be required to passthrough some iGPUs, so the passthrough of such devices can now work if the VGA is disabled. However, this is not in the scope of this PR, so only the
debug.enable.vgaenable/disable it's being handled for now.Limitations
The re-attach of framebuffer and VT terminals (once they were removed) might not work during runtime on some devices. This is a limitation observed with the device itself and not related to EVE. On such devices, reboot is required to get the video back. A small disclaimer is introduced in the documentation about this possible situation.
Tests
x86_64:
arm64