From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x233.google.com (mail-io0-x233.google.com [IPv6:2607:f8b0:4001:c06::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C093021E2DA43 for ; Wed, 16 Aug 2017 07:39:44 -0700 (PDT) Received: by mail-io0-x233.google.com with SMTP id j32so13727538iod.0 for ; Wed, 16 Aug 2017 07:42:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=u70meuIIAt781iYd2U8Z/PETHBPaoSKB+pihreQfS+w=; b=J9TYr/DGDnfQ955Sy3uzPIueKNF6sxr/tYO25RPv38p/UMHvWGPURMViafhvc/lEOv CwiO/yySWGOXoMgAnSyekArLBWliFbNQlzW0yotFrZFGF2ng1QIOqMbieewK8UeFkZvl B/9aP8S6YGVZOm2UVBHKMGWfg1RlTZvjHqPNg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=u70meuIIAt781iYd2U8Z/PETHBPaoSKB+pihreQfS+w=; b=FyqYWjwHDWWhynSSNq1qoE2brwLmNj6rb9UBJ2BSKsbuje7Myt2L0yQptJein92U2V DTJlAvS82e3eut1pVXdclrdEA2zS/FD/m06o7a3O3QgbpKHgi5rY6SuHi9BtDdHzUKVT QiRnQC0WWIv3f/wtOc6+9QG6tQKkprqdz8TVDWzGEOIJIW0nHktMQlh3Ng1ut4pa5xvo d39q8gugtNnAiTYfg/a6WxCqH9GDzAhDETIOFuJP5HvJxIZy0GeQAz0fxcIvOAlg9Pz+ VaqEUBHPTU8YJZ1TAlfwPZ6HiD4nTESRFJpVDERX1dSqtTqLdIFPp0hSdlCp/h9Agbcq aU0A== X-Gm-Message-State: AHYfb5iXG0XdfaRF7G354Fwz3Z/2jhuXwbqb9GWNmJw28PX65GGe6l9x 2amaFBr2g8gYSwPj3eLRT7iADUxnUQX9 X-Received: by 10.107.162.21 with SMTP id l21mr1659441ioe.154.1502894529807; Wed, 16 Aug 2017 07:42:09 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.162.1 with HTTP; Wed, 16 Aug 2017 07:42:09 -0700 (PDT) In-Reply-To: <20170816121040.15757-2-lersek@redhat.com> References: <20170816121040.15757-1-lersek@redhat.com> <20170816121040.15757-2-lersek@redhat.com> From: Ard Biesheuvel Date: Wed, 16 Aug 2017 15:42:09 +0100 Message-ID: To: Laszlo Ersek Cc: edk2-devel-01 , Heyi Guo , Star Zeng Subject: Re: [PATCH 1/1] ArmVirtPkg/FdtPL011SerialPortLib: call PL011UartLib in all SerialPortLib APIs X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Aug 2017 14:39:45 -0000 Content-Type: text/plain; charset="UTF-8" On 16 August 2017 at 13:10, Laszlo Ersek wrote: > With the SerialDxe change in commit 4cf3f37c87ba ("MdeModulePkg SerialDxe: > Process timeout consistently in SerialRead", 2017-07-18), setting > EFI_SERIAL_INPUT_BUFFER_EMPTY in the "Control" output parameter, in the > GetControl() SerialPortLib function, is no longer a "small optimization". > Namely, due to the SerialDxe change, the GetOneKeyFromSerial() call in > TerminalDxe's TerminalConInTimerHandler() can take very long if the input > queue is empty, even if GetOneKeyFromSerial()'s return value causes the > loop to be exited right after, in the first iteration. > > This issue causes a boot hang in ArmVirtQemu: with the input queue empty, > TerminalConInTimerHandler() takes so long to return that, by the time it > returns, there's another execution queued already (due to the associated > timer event being signaled meanwhile). The boot process is stuck in the > timer event handler. > > Therefore even the first GetOneKeyFromSerial() iteration must be prevented > in TerminalConInTimerHandler() if the input queue is empty, and that > requires implementing GetControl() for real. > > Implement the SetAttributes(), SetControl() and GetControl() APIs (of > SerialPortExtLib origin) in FdtPL011SerialPortLib with calls to matching > PL011UartLib functions. This follows the example of > "ArmPlatformPkg/Library/PL011SerialPortLib" and also matches Star's > original idea under [1]. > > The patch can be considered a continuation of commit ad7f6bc2e116 > ("ArmVirtPkg: Use SerialDxe in MdeModulePkg instead of EmbeddedPkg", > 2015-11-26), based on the mailing list threads [1] [2] [3]. > > [1] http://mid.mail-archive.com/1447752930-32880-12-git-send-email-star.zeng@intel.com > [2] http://mid.mail-archive.com/1448243067-1880-12-git-send-email-star.zeng@intel.com > [3] http://mid.mail-archive.com/b748580c-cb51-32c9-acf9-780841ef15da@redhat.com > > Cc: Ard Biesheuvel > Cc: Brijesh Singh > Cc: Heyi Guo > Cc: Star Zeng > Originally-suggested-by: Star Zeng > Reported-by: Brijesh Singh > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek Reviewed-by: Ard Biesheuvel Thanks Laszlo! > --- > ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c | 38 ++++++++++++++++++-- > 1 file changed, 35 insertions(+), 3 deletions(-) > > diff --git a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c > index 48a0530dcc2f..05d3547fda91 100644 > --- a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c > +++ b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c > @@ -200,7 +200,23 @@ SerialPortSetAttributes ( > IN OUT EFI_STOP_BITS_TYPE *StopBits > ) > { > - return RETURN_UNSUPPORTED; > + RETURN_STATUS Status; > + > + if (mSerialBaseAddress == 0) { > + Status = RETURN_UNSUPPORTED; > + } else { > + Status = PL011UartInitializePort ( > + mSerialBaseAddress, > + FixedPcdGet32 (PL011UartClkInHz), > + BaudRate, > + ReceiveFifoDepth, > + Parity, > + DataBits, > + StopBits > + ); > + } > + > + return Status; > } > > /** > @@ -219,7 +235,15 @@ SerialPortSetControl ( > IN UINT32 Control > ) > { > - return RETURN_UNSUPPORTED; > + RETURN_STATUS Status; > + > + if (mSerialBaseAddress == 0) { > + Status = RETURN_UNSUPPORTED; > + } else { > + Status = PL011UartSetControl (mSerialBaseAddress, Control); > + } > + > + return Status; > } > > /** > @@ -238,6 +262,14 @@ SerialPortGetControl ( > OUT UINT32 *Control > ) > { > - return RETURN_UNSUPPORTED; > + RETURN_STATUS Status; > + > + if (mSerialBaseAddress == 0) { > + Status = RETURN_UNSUPPORTED; > + } else { > + Status = PL011UartGetControl (mSerialBaseAddress, Control); > + } > + > + return Status; > } > > -- > 2.14.1.3.gb7cf6e02401b > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel