From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4B53421D490EC for ; Wed, 16 Aug 2017 17:59:58 -0700 (PDT) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Aug 2017 18:02:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,385,1498546800"; d="scan'208";a="890856035" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by FMSMGA003.fm.intel.com with ESMTP; 16 Aug 2017 18:02:10 -0700 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 16 Aug 2017 18:02:10 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx116.amr.corp.intel.com (10.18.116.20) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 16 Aug 2017 18:02:10 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.183]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.25]) with mapi id 14.03.0319.002; Thu, 17 Aug 2017 09:02:09 +0800 From: "Zeng, Star" To: Laszlo Ersek , edk2-devel-01 CC: Ard Biesheuvel , Brijesh Singh , Heyi Guo , "Zeng, Star" Thread-Topic: [PATCH 1/1] ArmVirtPkg/FdtPL011SerialPortLib: call PL011UartLib in all SerialPortLib APIs Thread-Index: AQHTFoi2oaxqem0UDkeNg10XrwCsIKKHu5hg Date: Thu, 17 Aug 2017 01:02:09 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B919A71@shsmsx102.ccr.corp.intel.com> References: <20170816121040.15757-1-lersek@redhat.com> <20170816121040.15757-2-lersek@redhat.com> In-Reply-To: <20170816121040.15757-2-lersek@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 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: Thu, 17 Aug 2017 00:59:58 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Very good commit log, history information and code change. :) Reviewed-by: Star Zeng Thanks, Star -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Wednesday, August 16, 2017 8:11 PM To: edk2-devel-01 Cc: Ard Biesheuvel ; Brijesh Singh ; Heyi Guo ; Zeng, Star Subject: [PATCH 1/1] ArmVirtPkg/FdtPL011SerialPortLib: call PL011UartLib in= all SerialPortLib APIs With the SerialDxe change in commit 4cf3f37c87ba ("MdeModulePkg SerialDxe: Process timeout consistently in SerialRead", 2017-07-18), setting EFI_SERIA= L_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 Term= inalDxe'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 re= turns, there's another execution queued already (due to the associated time= r event being signaled meanwhile). The boot process is stuck in the timer e= vent handler. Therefore even the first GetOneKeyFromSerial() iteration must be prevented = in TerminalConInTimerHandler() if the input queue is empty, and that requir= es implementing GetControl() for real. Implement the SetAttributes(), SetControl() and GetControl() APIs (of Seria= lPortExtLib origin) in FdtPL011SerialPortLib with calls to matching PL011Ua= rtLib functions. This follows the example of "ArmPlatformPkg/Library/PL011S= erialPortLib" 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-1= 1-26), based on the mailing list threads [1] [2] [3]. [1] http://mid.mail-archive.com/1447752930-32880-12-git-send-email-star.zen= g@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 --- 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 =3D=3D 0) { + Status =3D RETURN_UNSUPPORTED; + } else { + Status =3D PL011UartInitializePort ( + mSerialBaseAddress, + FixedPcdGet32 (PL011UartClkInHz), + BaudRate, + ReceiveFifoDepth, + Parity, + DataBits, + StopBits + ); + } + + return Status; } =20 /** @@ -219,7 +235,15 @@ SerialPortSetControl ( IN UINT32 Control ) { - return RETURN_UNSUPPORTED; + RETURN_STATUS Status; + + if (mSerialBaseAddress =3D=3D 0) { + Status =3D RETURN_UNSUPPORTED; + } else { + Status =3D PL011UartSetControl (mSerialBaseAddress, Control); } + + return Status; } =20 /** @@ -238,6 +262,14 @@ SerialPortGetControl ( OUT UINT32 *Control ) { - return RETURN_UNSUPPORTED; + RETURN_STATUS Status; + + if (mSerialBaseAddress =3D=3D 0) { + Status =3D RETURN_UNSUPPORTED; + } else { + Status =3D PL011UartGetControl (mSerialBaseAddress, Control); } + + return Status; } =20 -- 2.14.1.3.gb7cf6e02401b