On 13 May 2018 at 00:55, Laszlo Ersek <[email protected]> wrote: > If both ConIn and ConOut exist, but ConIn references none of the PS/2 > keyboard, the USB wild-card keyboard, and any serial ports, then > PlatformInitializeConsole() currently allows the boot to proceed without > any input devices at all. This makes for a bad user experience -- the > firmware menu could only be entered through OsIndications, set by a guest > OS. > > Do what ArmVirtQemu does already, namely connect the consoles, and add > them to ConIn / ConOut / ErrOut, unconditionally. (The underlying > EfiBootManagerUpdateConsoleVariable() function checks for duplicates.) > > The issue used to be masked by the EfiBootManagerConnectAll() call that > got conditionalized in commit 245c643cc8b7. > > This patch is best viewed with "git show -b -W". > > Cc: Ard Biesheuvel <[email protected]> > Cc: Jordan Justen <[email protected]> > Fixes: 245c643cc8b73240c3b88cb55b2911b285a8c10d > Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1577546 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]> > --- > > Notes: > Repo: https://github.com/lersek/edk2.git > Branch: conn_cons_uncond > > OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 127 > +++++++------------- > 1 file changed, 44 insertions(+), 83 deletions(-) > > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > index 862fa6ebb4e5..004b753f4d26 100644 > --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > @@ -26,7 +26,6 @@ VOID *mEfiDevPathNotifyReg; > EFI_EVENT mEfiDevPathEvent; > VOID *mEmuVariableEventReg; > EFI_EVENT mEmuVariableEvent; > -BOOLEAN mDetectVgaOnly; > UINT16 mHostBridgeDevId; > > // > @@ -830,35 +829,33 @@ DetectAndPreparePlatformPciDevicePath ( > ); > ASSERT_EFI_ERROR (Status); > > - if (!mDetectVgaOnly) { > + // > + // Here we decide whether it is LPC Bridge > + // > + if ((IS_PCI_LPC (Pci)) || > + ((IS_PCI_ISA_PDECODE (Pci)) && > + (Pci->Hdr.VendorId == 0x8086) && > + (Pci->Hdr.DeviceId == 0x7000) > + ) > + ) { > // > - // Here we decide whether it is LPC Bridge > + // Add IsaKeyboard to ConIn, > + // add IsaSerial to ConOut, ConIn, ErrOut > // > - if ((IS_PCI_LPC (Pci)) || > - ((IS_PCI_ISA_PDECODE (Pci)) && > - (Pci->Hdr.VendorId == 0x8086) && > - (Pci->Hdr.DeviceId == 0x7000) > - ) > - ) { > - // > - // Add IsaKeyboard to ConIn, > - // add IsaSerial to ConOut, ConIn, ErrOut > - // > - DEBUG ((EFI_D_INFO, "Found LPC Bridge device\n")); > - PrepareLpcBridgeDevicePath (Handle); > - return EFI_SUCCESS; > - } > + DEBUG ((EFI_D_INFO, "Found LPC Bridge device\n")); > + PrepareLpcBridgeDevicePath (Handle); > + return EFI_SUCCESS; > + } > + // > + // Here we decide which Serial device to enable in PCI bus > + // > + if (IS_PCI_16550SERIAL (Pci)) { > // > - // Here we decide which Serial device to enable in PCI bus > + // Add them to ConOut, ConIn, ErrOut. > // > - if (IS_PCI_16550SERIAL (Pci)) { > - // > - // Add them to ConOut, ConIn, ErrOut. > - // > - DEBUG ((EFI_D_INFO, "Found PCI 16550 SERIAL device\n")); > - PreparePciSerialDevicePath (Handle); > - return EFI_SUCCESS; > - } > + DEBUG ((EFI_D_INFO, "Found PCI 16550 SERIAL device\n")); > + PreparePciSerialDevicePath (Handle); > + return EFI_SUCCESS; > } > > // > @@ -877,26 +874,6 @@ DetectAndPreparePlatformPciDevicePath ( > } > > > -/** > - Do platform specific PCI Device check and add them to ConOut, ConIn, ErrOut > - > - @param[in] DetectVgaOnly - Only detect VGA device if it's TRUE. > - > - @retval EFI_SUCCESS - PCI Device check and Console variable update > - successfully. > - @retval EFI_STATUS - PCI Device check or Console variable update fail. > - > -**/ > -EFI_STATUS > -DetectAndPreparePlatformPciDevicePaths ( > - BOOLEAN DetectVgaOnly > - ) > -{ > - mDetectVgaOnly = DetectVgaOnly; > - return VisitAllPciInstances (DetectAndPreparePlatformPciDevicePath); > -} > - > - > /** > Connect the predefined platform default console device. > > @@ -910,50 +887,34 @@ PlatformInitializeConsole ( > ) > { > UINTN Index; > - EFI_DEVICE_PATH_PROTOCOL *VarConout; > - EFI_DEVICE_PATH_PROTOCOL *VarConin; > > // > - // Connect RootBridge > + // Do platform specific PCI Device check and add them to ConOut, ConIn, > + // ErrOut > // > - GetEfiGlobalVariable2 (EFI_CON_OUT_VARIABLE_NAME, (VOID **) &VarConout, > - NULL); > - GetEfiGlobalVariable2 (EFI_CON_IN_VARIABLE_NAME, (VOID **) &VarConin, > NULL); > - > - if (VarConout == NULL || VarConin == NULL) { > - // > - // Do platform specific PCI Device check and add them to ConOut, ConIn, > - // ErrOut > - // > - DetectAndPreparePlatformPciDevicePaths (FALSE); > + VisitAllPciInstances (DetectAndPreparePlatformPciDevicePath); > > + // > + // Have chance to connect the platform default console, > + // the platform default console is the minimum device group > + // the platform should support > + // > + for (Index = 0; PlatformConsole[Index].DevicePath != NULL; ++Index) { > // > - // Have chance to connect the platform default console, > - // the platform default console is the minimum device group > - // the platform should support > + // Update the console variable with the connect type > // > - for (Index = 0; PlatformConsole[Index].DevicePath != NULL; ++Index) { > - // > - // Update the console variable with the connect type > - // > - if ((PlatformConsole[Index].ConnectType & CONSOLE_IN) == CONSOLE_IN) { > - EfiBootManagerUpdateConsoleVariable (ConIn, > - PlatformConsole[Index].DevicePath, NULL); > - } > - if ((PlatformConsole[Index].ConnectType & CONSOLE_OUT) == CONSOLE_OUT) > { > - EfiBootManagerUpdateConsoleVariable (ConOut, > - PlatformConsole[Index].DevicePath, NULL); > - } > - if ((PlatformConsole[Index].ConnectType & STD_ERROR) == STD_ERROR) { > - EfiBootManagerUpdateConsoleVariable (ErrOut, > - PlatformConsole[Index].DevicePath, NULL); > - } > + if ((PlatformConsole[Index].ConnectType & CONSOLE_IN) == CONSOLE_IN) { > + EfiBootManagerUpdateConsoleVariable (ConIn, > + PlatformConsole[Index].DevicePath, NULL); > + } > + if ((PlatformConsole[Index].ConnectType & CONSOLE_OUT) == CONSOLE_OUT) { > + EfiBootManagerUpdateConsoleVariable (ConOut, > + PlatformConsole[Index].DevicePath, NULL); > + } > + if ((PlatformConsole[Index].ConnectType & STD_ERROR) == STD_ERROR) { > + EfiBootManagerUpdateConsoleVariable (ErrOut, > + PlatformConsole[Index].DevicePath, NULL); > } > - } else { > - // > - // Only detect VGA device and add them to ConOut > - // > - DetectAndPreparePlatformPciDevicePaths (TRUE); > } > } > > -- > 2.14.1.3.gb7cf6e02401b > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

