public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Heyi Guo <guoheyi@huawei.com>, Star Zeng <star.zeng@intel.com>
Subject: [PATCH 1/1] ArmVirtPkg/FdtPL011SerialPortLib: call PL011UartLib in all SerialPortLib APIs
Date: Wed, 16 Aug 2017 14:10:40 +0200	[thread overview]
Message-ID: <20170816121040.15757-2-lersek@redhat.com> (raw)
In-Reply-To: <20170816121040.15757-1-lersek@redhat.com>

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 <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Heyi Guo <guoheyi@huawei.com>
Cc: Star Zeng <star.zeng@intel.com>
Originally-suggested-by: Star Zeng <star.zeng@intel.com>
Reported-by: Brijesh Singh <brijesh.singh@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 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



  reply	other threads:[~2017-08-16 12:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-16 12:10 [PATCH 0/1] ArmVirtPkg/FdtPL011SerialPortLib: call PL011UartLib in all SerialPortLib APIs Laszlo Ersek
2017-08-16 12:10 ` Laszlo Ersek [this message]
2017-08-16 14:42   ` [PATCH 1/1] " Ard Biesheuvel
2017-08-16 15:03   ` Brijesh Singh
2017-08-16 15:36     ` Laszlo Ersek
2017-08-17  1:02   ` Zeng, Star
2017-08-17 12:17     ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170816121040.15757-2-lersek@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox