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

Reply via email to