[PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

[PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

[Star Zeng]

https://lists.01.org/pipermail/edk2-devel/2017-July/012385.html
reported the timeout processing in SerialRead is not consistent.

Since SerialPortPoll only checks the status of serial port and
returns immediately, and SerialPortRead does not really implement
a time out mechanism and will always wait for enough input,
it will cause below results:
1. If there is no serial input at all, this interface will return
timeout immediately without any waiting;
2. If there is A characters in serial port FIFO, and caller requires
A+1 characters, it will wait until a new input is coming and timeout
will not really occur.

This patch is to update SerialRead() to check SerialPortPoll() and
read data through SerialPortRead() one byte by one byte, and check
timeout against mSerialIoMode.Timeout if no input.

Cc: Heyi Guo <heyi.guo@linaro.org>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Universal/SerialDxe/SerialIo.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
index d2383e56dd8f..43d33dba0c2a 100644
--- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
+++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
@@ -465,11 +465,25 @@ SerialRead (
   )
 {
   UINTN Count;
+  UINTN TimeOut;
 
   Count = 0;
 
-  if (SerialPortPoll ()) {
-    Count = SerialPortRead (Buffer, *BufferSize);
+  while (Count < *BufferSize) {
+    TimeOut = 0;
+    while (TimeOut < mSerialIoMode.Timeout) {
+      if (SerialPortPoll ()) {
+        break;
+      }
+      gBS->Stall (10);
+      TimeOut += 10;
+    }
+    if (TimeOut >= mSerialIoMode.Timeout) {
+      break;
+    }
+    SerialPortRead (Buffer, 1);
+    Count++;
+    Buffer = (VOID *) ((UINT8 *) Buffer + 1);
   }
 
   if (Count != *BufferSize) {
-- 
2.7.0.windows.1

Re: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

[Ni, Ruiyu]

Star,
3 minor comments below.

Thanks/Ray

> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, August 4, 2017 4:29 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Heyi Guo <heyi.guo@linaro.org>; Ni,
> Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently in
> SerialRead
> 
> https://lists.01.org/pipermail/edk2-devel/2017-July/012385.html
> reported the timeout processing in SerialRead is not consistent.
> 
> Since SerialPortPoll only checks the status of serial port and returns
> immediately, and SerialPortRead does not really implement a time out
> mechanism and will always wait for enough input, it will cause below results:
> 1. If there is no serial input at all, this interface will return timeout
> immediately without any waiting; 2. If there is A characters in serial port FIFO,
> and caller requires
> A+1 characters, it will wait until a new input is coming and timeout
> will not really occur.
> 
> This patch is to update SerialRead() to check SerialPortPoll() and read data
> through SerialPortRead() one byte by one byte, and check timeout against
> mSerialIoMode.Timeout if no input.
> 
> Cc: Heyi Guo <heyi.guo@linaro.org>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Universal/SerialDxe/SerialIo.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> index d2383e56dd8f..43d33dba0c2a 100644
> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> @@ -465,11 +465,25 @@ SerialRead (
>    )
>  {
>    UINTN Count;
> +  UINTN TimeOut;
> 
>    Count = 0;
> 
> -  if (SerialPortPoll ()) {
> -    Count = SerialPortRead (Buffer, *BufferSize);
> +  while (Count < *BufferSize) {
> +    TimeOut = 0;
> +    while (TimeOut < mSerialIoMode.Timeout) {
> +      if (SerialPortPoll ()) {
> +        break;
> +      }
> +      gBS->Stall (10);
1. can you use EFI_TIMER_PERIOD_MICROSECONDS(1)?

> +      TimeOut += 10;
2. TImeOut++?

> +    }
> +    if (TimeOut >= mSerialIoMode.Timeout) {
3. if (TimeOut == ...) { ?

> +      break;
> +    }
> +    SerialPortRead (Buffer, 1);
> +    Count++;
> +    Buffer = (VOID *) ((UINT8 *) Buffer + 1);
>    }
> 
>    if (Count != *BufferSize) {
> --
> 2.7.0.windows.1

Re: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

[Zeng, Star]

Thanks for the comments.

EFI_TIMER_PERIOD_MICROSECONDS is used for timer event according to its definition, and its unit is 100ns.
But the unit of mSerialIoMode.Timeout and gBS->Stall() is 1us.

+1 may cause more polling of SerialPortPoll(). How about keeping using +10? :)


Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu 
Sent: Friday, August 4, 2017 5:13 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Heyi Guo <heyi.guo@linaro.org>; Laszlo Ersek <lersek@redhat.com>
Subject: RE: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

Star,
3 minor comments below.

Thanks/Ray

> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, August 4, 2017 4:29 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Heyi Guo <heyi.guo@linaro.org>; 
> Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently 
> in SerialRead
> 
> https://lists.01.org/pipermail/edk2-devel/2017-July/012385.html
> reported the timeout processing in SerialRead is not consistent.
> 
> Since SerialPortPoll only checks the status of serial port and returns 
> immediately, and SerialPortRead does not really implement a time out 
> mechanism and will always wait for enough input, it will cause below results:
> 1. If there is no serial input at all, this interface will return 
> timeout immediately without any waiting; 2. If there is A characters 
> in serial port FIFO, and caller requires
> A+1 characters, it will wait until a new input is coming and timeout
> will not really occur.
> 
> This patch is to update SerialRead() to check SerialPortPoll() and 
> read data through SerialPortRead() one byte by one byte, and check 
> timeout against mSerialIoMode.Timeout if no input.
> 
> Cc: Heyi Guo <heyi.guo@linaro.org>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Universal/SerialDxe/SerialIo.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> index d2383e56dd8f..43d33dba0c2a 100644
> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> @@ -465,11 +465,25 @@ SerialRead (
>    )
>  {
>    UINTN Count;
> +  UINTN TimeOut;
> 
>    Count = 0;
> 
> -  if (SerialPortPoll ()) {
> -    Count = SerialPortRead (Buffer, *BufferSize);
> +  while (Count < *BufferSize) {
> +    TimeOut = 0;
> +    while (TimeOut < mSerialIoMode.Timeout) {
> +      if (SerialPortPoll ()) {
> +        break;
> +      }
> +      gBS->Stall (10);
1. can you use EFI_TIMER_PERIOD_MICROSECONDS(1)?

> +      TimeOut += 10;
2. TImeOut++?

> +    }
> +    if (TimeOut >= mSerialIoMode.Timeout) {
3. if (TimeOut == ...) { ?

> +      break;
> +    }
> +    SerialPortRead (Buffer, 1);
> +    Count++;
> +    Buffer = (VOID *) ((UINT8 *) Buffer + 1);
>    }
> 
>    if (Count != *BufferSize) {
> --
> 2.7.0.windows.1

Re: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

[Ni, Ruiyu]

I thought unit of Stall is 100ns. Then no issues now.

Reviewed-by: Ruiyu Ni <Ruiyu.ni@intel.com>

Thanks/Ray

> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, August 4, 2017 5:25 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Heyi Guo <heyi.guo@linaro.org>; Laszlo Ersek <lersek@redhat.com>;
> Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently
> in SerialRead
> 
> Thanks for the comments.
> 
> EFI_TIMER_PERIOD_MICROSECONDS is used for timer event according to its
> definition, and its unit is 100ns.
> But the unit of mSerialIoMode.Timeout and gBS->Stall() is 1us.
> 
> +1 may cause more polling of SerialPortPoll(). How about keeping using
> ++10? :)
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Friday, August 4, 2017 5:13 PM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Heyi Guo <heyi.guo@linaro.org>; Laszlo Ersek <lersek@redhat.com>
> Subject: RE: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently
> in SerialRead
> 
> Star,
> 3 minor comments below.
> 
> Thanks/Ray
> 
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Friday, August 4, 2017 4:29 PM
> > To: edk2-devel@lists.01.org
> > Cc: Zeng, Star <star.zeng@intel.com>; Heyi Guo <heyi.guo@linaro.org>;
> > Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> > Subject: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently
> > in SerialRead
> >
> > https://lists.01.org/pipermail/edk2-devel/2017-July/012385.html
> > reported the timeout processing in SerialRead is not consistent.
> >
> > Since SerialPortPoll only checks the status of serial port and returns
> > immediately, and SerialPortRead does not really implement a time out
> > mechanism and will always wait for enough input, it will cause below results:
> > 1. If there is no serial input at all, this interface will return
> > timeout immediately without any waiting; 2. If there is A characters
> > in serial port FIFO, and caller requires
> > A+1 characters, it will wait until a new input is coming and timeout
> > will not really occur.
> >
> > This patch is to update SerialRead() to check SerialPortPoll() and
> > read data through SerialPortRead() one byte by one byte, and check
> > timeout against mSerialIoMode.Timeout if no input.
> >
> > Cc: Heyi Guo <heyi.guo@linaro.org>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > ---
> >  MdeModulePkg/Universal/SerialDxe/SerialIo.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> > b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> > index d2383e56dd8f..43d33dba0c2a 100644
> > --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> > +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> > @@ -465,11 +465,25 @@ SerialRead (
> >    )
> >  {
> >    UINTN Count;
> > +  UINTN TimeOut;
> >
> >    Count = 0;
> >
> > -  if (SerialPortPoll ()) {
> > -    Count = SerialPortRead (Buffer, *BufferSize);
> > +  while (Count < *BufferSize) {
> > +    TimeOut = 0;
> > +    while (TimeOut < mSerialIoMode.Timeout) {
> > +      if (SerialPortPoll ()) {
> > +        break;
> > +      }
> > +      gBS->Stall (10);
> 1. can you use EFI_TIMER_PERIOD_MICROSECONDS(1)?
> 
> > +      TimeOut += 10;
> 2. TImeOut++?
> 
> > +    }
> > +    if (TimeOut >= mSerialIoMode.Timeout) {
> 3. if (TimeOut == ...) { ?
> 
> > +      break;
> > +    }
> > +    SerialPortRead (Buffer, 1);
> > +    Count++;
> > +    Buffer = (VOID *) ((UINT8 *) Buffer + 1);
> >    }
> >
> >    if (Count != *BufferSize) {
> > --
> > 2.7.0.windows.1

Re: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

[Laszlo Ersek]

Hi Star,

On 08/04/17 10:29, Star Zeng wrote:
> https://lists.01.org/pipermail/edk2-devel/2017-July/012385.html
> reported the timeout processing in SerialRead is not consistent.
>
> Since SerialPortPoll only checks the status of serial port and
> returns immediately, and SerialPortRead does not really implement
> a time out mechanism and will always wait for enough input,
> it will cause below results:
> 1. If there is no serial input at all, this interface will return
> timeout immediately without any waiting;
> 2. If there is A characters in serial port FIFO, and caller requires
> A+1 characters, it will wait until a new input is coming and timeout
> will not really occur.
>
> This patch is to update SerialRead() to check SerialPortPoll() and
> read data through SerialPortRead() one byte by one byte, and check
> timeout against mSerialIoMode.Timeout if no input.
>
> Cc: Heyi Guo <heyi.guo@linaro.org>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Universal/SerialDxe/SerialIo.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> index d2383e56dd8f..43d33dba0c2a 100644
> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> @@ -465,11 +465,25 @@ SerialRead (
>    )
>  {
>    UINTN Count;
> +  UINTN TimeOut;
>
>    Count = 0;
>
> -  if (SerialPortPoll ()) {
> -    Count = SerialPortRead (Buffer, *BufferSize);
> +  while (Count < *BufferSize) {
> +    TimeOut = 0;
> +    while (TimeOut < mSerialIoMode.Timeout) {
> +      if (SerialPortPoll ()) {
> +        break;
> +      }
> +      gBS->Stall (10);
> +      TimeOut += 10;
> +    }
> +    if (TimeOut >= mSerialIoMode.Timeout) {
> +      break;
> +    }
> +    SerialPortRead (Buffer, 1);
> +    Count++;
> +    Buffer = (VOID *) ((UINT8 *) Buffer + 1);
>    }
>
>    if (Count != *BufferSize) {
>

This patch breaks the ArmVirtQemu platform's boot process -- it seems to
fall into an infinite loop. I guess the above loop(s) never complete?

If I revert this patch on top of current master (af0364f01e8c,
"Nt32/PlatformBootManagerLib: Enable STD_ERROR on all consoles",
2017-08-14), then ArmVirtQemu boots fine.

I found this commit with bisection:

> 4cf3f37c87ba1f9d58072444bd735e40e4779e70 is the first bad commit
> commit 4cf3f37c87ba1f9d58072444bd735e40e4779e70
> Author: Star Zeng <star.zeng@intel.com>
> Date:   Tue Jul 18 16:32:16 2017 +0800
>
>     MdeModulePkg SerialDxe: Process timeout consistently in SerialRead
>
>     https://lists.01.org/pipermail/edk2-devel/2017-July/012385.html
>     reported the timeout processing in SerialRead is not consistent.
>
>     Since SerialPortPoll only checks the status of serial port and
>     returns immediately, and SerialPortRead does not really implement
>     a time out mechanism and will always wait for enough input,
>     it will cause below results:
>     1. If there is no serial input at all, this interface will return
>     timeout immediately without any waiting;
>     2. If there is A characters in serial port FIFO, and caller requires
>     A+1 characters, it will wait until a new input is coming and timeout
>     will not really occur.
>
>     This patch is to update SerialRead() to check SerialPortPoll() and
>     read data through SerialPortRead() one byte by one byte, and check
>     timeout against mSerialIoMode.Timeout if no input.
>
>     Cc: Heyi Guo <heyi.guo@linaro.org>
>     Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>     Cc: Laszlo Ersek <lersek@redhat.com>
>     Contributed-under: TianoCore Contribution Agreement 1.0
>     Signed-off-by: Star Zeng <star.zeng@intel.com>
>     Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
>
> :040000 040000 aaa8b75d378ec613b828db938c36a16723583906 d034044918d7f5b8d30783e0a9eccdf3cf21e5c5 M      MdeModulePkg

Bisection log:

> git bisect start
> # bad: [af0364f01e8cac95afad01437f13beef90f6640b] Nt32/PlatformBootManagerLib: Enable STD_ERROR on all consoles
> git bisect bad af0364f01e8cac95afad01437f13beef90f6640b
> # good: [c325e41585e374d40fb36b434e61a1ab0fca5b1c] MdeModulePkg: Fix coding style issues
> git bisect good c325e41585e374d40fb36b434e61a1ab0fca5b1c
> # good: [636cda51903b4b28e1c9b099c4f22a84e61b09da] OvmfPkg/OvmfPkg.fdf.inc: extract VARS_LIVE_SIZE and VARS_SPARE_SIZE macros
> git bisect good 636cda51903b4b28e1c9b099c4f22a84e61b09da
> # good: [997b2c543751cb4a3473270c1a7016ade311f01b] BaseTools/GenCrc32: Fix a bug to hand empty file for decode
> git bisect good 997b2c543751cb4a3473270c1a7016ade311f01b
> # good: [f1658838c267723139711c0b15d98a74980ae4c5] OvmfPkg/IoMmuDxe: abort harder on memory encryption mask failures
> git bisect good f1658838c267723139711c0b15d98a74980ae4c5
> # bad: [9458afa33728e64049d465f052b2c5c3ca3e881c] IntelFrameworkModulePkg: Fix Xcode 9 Beta treating 32-bit left shift as undefined
> git bisect bad 9458afa33728e64049d465f052b2c5c3ca3e881c
> # good: [6e414300b5f19d3045a0d21ad90ac2fe965478a5] EmbeddedPkg/AndroidFastboot: split android boot header
> git bisect good 6e414300b5f19d3045a0d21ad90ac2fe965478a5
> # bad: [416d48f755518fd1d202b97be2e9944df6e8f0d4] ShellPkg/drivers: Show Image Name in non-SFO mode
> git bisect bad 416d48f755518fd1d202b97be2e9944df6e8f0d4
> # bad: [2913ebb2b550f50a14f105e26995dd095e627ba4] NetworkPkg/HttpBootDxe: Refine the coding style.
> git bisect bad 2913ebb2b550f50a14f105e26995dd095e627ba4
> # bad: [9e2a8e928995c3b1bb664b73fd59785055c6b5f6] OvmfPkg/AcpiPlatformDxe: short-circuit the transfer of an empty S3_CONTEXT
> git bisect bad 9e2a8e928995c3b1bb664b73fd59785055c6b5f6
> # bad: [4cf3f37c87ba1f9d58072444bd735e40e4779e70] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead
> git bisect bad 4cf3f37c87ba1f9d58072444bd735e40e4779e70
> # first bad commit: [4cf3f37c87ba1f9d58072444bd735e40e4779e70] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

In retrospect, I see that you asked me for feedback in the original
discussion, at the following URL:

  https://lists.01.org/pipermail/edk2-devel/2017-July/012406.html

Unfortunately, this was on July 18th, and I was on vacation then. (I
think I configured my email account to send an automated out-of-office
reply.)

Looking at the patch now, one idea I have is to keep the time running
across all bytes read; that is, use mSerialIoMode.Timeout as a global
timeout for the entire SerialRead() call.

Unfortunately, the UEFI-2.7 spec defines the "SERIAL_IO_MODE.Timeout"
field, and the Timeout parameter of
EFI_SERIAL_IO_PROTOCOL.SetAttributes(), inconsistently with each other.
First it says (about the field):

  Timeout  If applicable, the number of microseconds to wait before
           timing out a Read or Write operation.

(This is compatible with my idea.)

But then it says (in the general description, and about the
SetAttributes() parameter):

  The default attributes for all UART-style serial device interfaces
  are: [...] a 1,000,000 microsecond timeout *per character* [...]

  Timeout  The requested time out *for a single character* in
           microseconds. This timeout applies to both the transmit and
           receive side of the interface. A Timeout value of 0 will use
           the device's default time out value.

(Emphases mine.)

... I added some debug messages to the loops, and the first invocation
of the function seems to hang with the following parameters:

> SerialRead: BufferSize=1 mSerialIoMode.Timeout=1000000

That is, the caller wants to read a single character (which is not
arriving). The timeout is the default 1 second. However, the wait takes
much-much longer than 1 second (it appears to be infinite). It seems
that gBS->Stall(10) takes much longer than 10 microseconds. According to
the UEFI spec,

  The Stall() function stalls execution on the processor for at least
  the requested number of microseconds. [...]

So Stall() is allowed to wait longer (possibly a lot longer) -- I guess
it depends on some timer's resolution.

I have another idea related to this -- let me research it a bit later. I
might post some patches.

Thanks
Laszlo

Re: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

[Kinney, Michael D]

Laszlo,

gBS->Stall() layers on top of the Metronome Architectural Protocol.

armvirtqemu.dsc:  EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf

And this implementation layers on top of a TimerLib

armvirtqemu.dsc:  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf

There are a couple PCDs involved in this module and lib. Maybe the
ArmVirtPkg needs to set some different PCD values to get a more accurate
gBS->Stall() when running through QEMU.

Best regards,

Mike


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> Behalf Of Laszlo Ersek
> Sent: Tuesday, August 15, 2017 4:31 PM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Heyi Guo
> <heyi.guo@linaro.org>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [PATCH] MdeModulePkg SerialDxe: Process
> timeout consistently in SerialRead
> 
> Hi Star,
> 
> On 08/04/17 10:29, Star Zeng wrote:
> > https://lists.01.org/pipermail/edk2-devel/2017-
> July/012385.html
> > reported the timeout processing in SerialRead is not
> consistent.
> >
> > Since SerialPortPoll only checks the status of serial port and
> > returns immediately, and SerialPortRead does not really
> implement
> > a time out mechanism and will always wait for enough input,
> > it will cause below results:
> > 1. If there is no serial input at all, this interface will
> return
> > timeout immediately without any waiting;
> > 2. If there is A characters in serial port FIFO, and caller
> requires
> > A+1 characters, it will wait until a new input is coming and
> timeout
> > will not really occur.
> >
> > This patch is to update SerialRead() to check SerialPortPoll()
> and
> > read data through SerialPortRead() one byte by one byte, and
> check
> > timeout against mSerialIoMode.Timeout if no input.
> >
> > Cc: Heyi Guo <heyi.guo@linaro.org>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > ---
> >  MdeModulePkg/Universal/SerialDxe/SerialIo.c | 18
> ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> > index d2383e56dd8f..43d33dba0c2a 100644
> > --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> > +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> > @@ -465,11 +465,25 @@ SerialRead (
> >    )
> >  {
> >    UINTN Count;
> > +  UINTN TimeOut;
> >
> >    Count = 0;
> >
> > -  if (SerialPortPoll ()) {
> > -    Count = SerialPortRead (Buffer, *BufferSize);
> > +  while (Count < *BufferSize) {
> > +    TimeOut = 0;
> > +    while (TimeOut < mSerialIoMode.Timeout) {
> > +      if (SerialPortPoll ()) {
> > +        break;
> > +      }
> > +      gBS->Stall (10);
> > +      TimeOut += 10;
> > +    }
> > +    if (TimeOut >= mSerialIoMode.Timeout) {
> > +      break;
> > +    }
> > +    SerialPortRead (Buffer, 1);
> > +    Count++;
> > +    Buffer = (VOID *) ((UINT8 *) Buffer + 1);
> >    }
> >
> >    if (Count != *BufferSize) {
> >
> 
> This patch breaks the ArmVirtQemu platform's boot process -- it
> seems to
> fall into an infinite loop. I guess the above loop(s) never
> complete?
> 
> If I revert this patch on top of current master (af0364f01e8c,
> "Nt32/PlatformBootManagerLib: Enable STD_ERROR on all consoles",
> 2017-08-14), then ArmVirtQemu boots fine.
> 
> I found this commit with bisection:
> 
> > 4cf3f37c87ba1f9d58072444bd735e40e4779e70 is the first bad
> commit
> > commit 4cf3f37c87ba1f9d58072444bd735e40e4779e70
> > Author: Star Zeng <star.zeng@intel.com>
> > Date:   Tue Jul 18 16:32:16 2017 +0800
> >
> >     MdeModulePkg SerialDxe: Process timeout consistently in
> SerialRead
> >
> >     https://lists.01.org/pipermail/edk2-devel/2017-
> July/012385.html
> >     reported the timeout processing in SerialRead is not
> consistent.
> >
> >     Since SerialPortPoll only checks the status of serial port
> and
> >     returns immediately, and SerialPortRead does not really
> implement
> >     a time out mechanism and will always wait for enough
> input,
> >     it will cause below results:
> >     1. If there is no serial input at all, this interface will
> return
> >     timeout immediately without any waiting;
> >     2. If there is A characters in serial port FIFO, and
> caller requires
> >     A+1 characters, it will wait until a new input is coming
> and timeout
> >     will not really occur.
> >
> >     This patch is to update SerialRead() to check
> SerialPortPoll() and
> >     read data through SerialPortRead() one byte by one byte,
> and check
> >     timeout against mSerialIoMode.Timeout if no input.
> >
> >     Cc: Heyi Guo <heyi.guo@linaro.org>
> >     Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> >     Cc: Laszlo Ersek <lersek@redhat.com>
> >     Contributed-under: TianoCore Contribution Agreement 1.0
> >     Signed-off-by: Star Zeng <star.zeng@intel.com>
> >     Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
> >
> > :040000 040000 aaa8b75d378ec613b828db938c36a16723583906
> d034044918d7f5b8d30783e0a9eccdf3cf21e5c5 M      MdeModulePkg
> 
> Bisection log:
> 
> > git bisect start
> > # bad: [af0364f01e8cac95afad01437f13beef90f6640b]
> Nt32/PlatformBootManagerLib: Enable STD_ERROR on all consoles
> > git bisect bad af0364f01e8cac95afad01437f13beef90f6640b
> > # good: [c325e41585e374d40fb36b434e61a1ab0fca5b1c]
> MdeModulePkg: Fix coding style issues
> > git bisect good c325e41585e374d40fb36b434e61a1ab0fca5b1c
> > # good: [636cda51903b4b28e1c9b099c4f22a84e61b09da]
> OvmfPkg/OvmfPkg.fdf.inc: extract VARS_LIVE_SIZE and
> VARS_SPARE_SIZE macros
> > git bisect good 636cda51903b4b28e1c9b099c4f22a84e61b09da
> > # good: [997b2c543751cb4a3473270c1a7016ade311f01b]
> BaseTools/GenCrc32: Fix a bug to hand empty file for decode
> > git bisect good 997b2c543751cb4a3473270c1a7016ade311f01b
> > # good: [f1658838c267723139711c0b15d98a74980ae4c5]
> OvmfPkg/IoMmuDxe: abort harder on memory encryption mask
> failures
> > git bisect good f1658838c267723139711c0b15d98a74980ae4c5
> > # bad: [9458afa33728e64049d465f052b2c5c3ca3e881c]
> IntelFrameworkModulePkg: Fix Xcode 9 Beta treating 32-bit left
> shift as undefined
> > git bisect bad 9458afa33728e64049d465f052b2c5c3ca3e881c
> > # good: [6e414300b5f19d3045a0d21ad90ac2fe965478a5]
> EmbeddedPkg/AndroidFastboot: split android boot header
> > git bisect good 6e414300b5f19d3045a0d21ad90ac2fe965478a5
> > # bad: [416d48f755518fd1d202b97be2e9944df6e8f0d4]
> ShellPkg/drivers: Show Image Name in non-SFO mode
> > git bisect bad 416d48f755518fd1d202b97be2e9944df6e8f0d4
> > # bad: [2913ebb2b550f50a14f105e26995dd095e627ba4]
> NetworkPkg/HttpBootDxe: Refine the coding style.
> > git bisect bad 2913ebb2b550f50a14f105e26995dd095e627ba4
> > # bad: [9e2a8e928995c3b1bb664b73fd59785055c6b5f6]
> OvmfPkg/AcpiPlatformDxe: short-circuit the transfer of an empty
> S3_CONTEXT
> > git bisect bad 9e2a8e928995c3b1bb664b73fd59785055c6b5f6
> > # bad: [4cf3f37c87ba1f9d58072444bd735e40e4779e70] MdeModulePkg
> SerialDxe: Process timeout consistently in SerialRead
> > git bisect bad 4cf3f37c87ba1f9d58072444bd735e40e4779e70
> > # first bad commit: [4cf3f37c87ba1f9d58072444bd735e40e4779e70]
> MdeModulePkg SerialDxe: Process timeout consistently in
> SerialRead
> 
> In retrospect, I see that you asked me for feedback in the
> original
> discussion, at the following URL:
> 
>   https://lists.01.org/pipermail/edk2-devel/2017-
> July/012406.html
> 
> Unfortunately, this was on July 18th, and I was on vacation
> then. (I
> think I configured my email account to send an automated out-of-
> office
> reply.)
> 
> Looking at the patch now, one idea I have is to keep the time
> running
> across all bytes read; that is, use mSerialIoMode.Timeout as a
> global
> timeout for the entire SerialRead() call.
> 
> Unfortunately, the UEFI-2.7 spec defines the
> "SERIAL_IO_MODE.Timeout"
> field, and the Timeout parameter of
> EFI_SERIAL_IO_PROTOCOL.SetAttributes(), inconsistently with each
> other.
> First it says (about the field):
> 
>   Timeout  If applicable, the number of microseconds to wait
> before
>            timing out a Read or Write operation.
> 
> (This is compatible with my idea.)
> 
> But then it says (in the general description, and about the
> SetAttributes() parameter):
> 
>   The default attributes for all UART-style serial device
> interfaces
>   are: [...] a 1,000,000 microsecond timeout *per character*
> [...]
> 
>   Timeout  The requested time out *for a single character* in
>            microseconds. This timeout applies to both the
> transmit and
>            receive side of the interface. A Timeout value of 0
> will use
>            the device's default time out value.
> 
> (Emphases mine.)
> 
> ... I added some debug messages to the loops, and the first
> invocation
> of the function seems to hang with the following parameters:
> 
> > SerialRead: BufferSize=1 mSerialIoMode.Timeout=1000000
> 
> That is, the caller wants to read a single character (which is
> not
> arriving). The timeout is the default 1 second. However, the
> wait takes
> much-much longer than 1 second (it appears to be infinite). It
> seems
> that gBS->Stall(10) takes much longer than 10 microseconds.
> According to
> the UEFI spec,
> 
>   The Stall() function stalls execution on the processor for at
> least
>   the requested number of microseconds. [...]
> 
> So Stall() is allowed to wait longer (possibly a lot longer) --
> I guess
> it depends on some timer's resolution.
> 
> I have another idea related to this -- let me research it a bit
> later. I
> might post some patches.
> 
> Thanks
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

[Laszlo Ersek]

On 08/16/17 01:59, Kinney, Michael D wrote:
> Laszlo,
> 
> gBS->Stall() layers on top of the Metronome Architectural Protocol.
> 
> armvirtqemu.dsc:  EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
> 
> And this implementation layers on top of a TimerLib
> 
> armvirtqemu.dsc:  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> 
> There are a couple PCDs involved in this module and lib. Maybe the
> ArmVirtPkg needs to set some different PCD values to get a more accurate
> gBS->Stall() when running through QEMU.

The issue is different; I checked gBS->Stall() in the above context and
it waits for the appropriate time (usually just 1-2 microseconds more
than requested).

Instead, the following happens:

- TerminalConInTimerHandler() in
"MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c" calls
SerialIo->GetControl().

- If the GetControl() call fails (for any reason), or the returned
Control word has EFI_SERIAL_INPUT_BUFFER_EMPTY clear, then
TerminalConInTimerHandler() enters a loop:

    //
    // Fetch all the keys in the serial buffer,
    // and insert the byte stream into RawFIFO.
    //

- The loop body calls GetOneKeyFromSerial() --> SerialIo->Read(), with a
1 byte buffer.

- The loop runs until the "raw data FIFO buffer" is filled completely
(256 byte is the size, apparently -- RAW_FIFO_MAX_NUMBER), or
GetOneKeyFromSerial() returns an error.

- The SerialPortGetControl() function in
"ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c"
returns constant RETURN_UNSUPPORTED. According to the library class
header "MdePkg/Include/Library/SerialPortLib.h", this is a valid thing
to do ("The serial device does not support this operation"). However, it
will cause TerminalConInTimerHandler() to *always* enter the loop, even
if there is no pending data to read.

- Because the input queue is empty, GetOneKeyFromSerial() will take a
full second before it times out.

- And the final piece of the puzzle is that the event associated with
TerminalConInTimerHandler() is signaled at 50Hz (0.02s period, see
KEYBOARD_TIMER_INTERVAL), initially. It can dynamically adjust its
frequency to the serial device's timeout, but in practice, as soon as
the current TerminalConInTimerHandler() frame returns, there's
(apparently) another execution queued already. So basically we're stuck
in the timer event handler.

I think we should implement the missing ("unsupported") functions in
"ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c" and
possibly in
"ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c".
I believe we could use
"ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c" as an
example.

Thanks
Laszlo

Re: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

[Zeng, Star]

  *Control = 0;
  if (!SerialPortPoll ()) {
    *Control = EFI_SERIAL_INPUT_BUFFER_EMPTY;
  }
  return RETURN_SUCCESS;

As example, the code above (in OvmfPkg\Library\XenConsoleSerialPortLib\XenConsoleSerialPortLib.c) can be simply used in SerialPortGetControl().


Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Wednesday, August 16, 2017 10:02 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Heyi Guo <heyi.guo@linaro.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [edk2] [PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

On 08/16/17 01:59, Kinney, Michael D wrote:
> Laszlo,
> 
> gBS->Stall() layers on top of the Metronome Architectural Protocol.
> 
> armvirtqemu.dsc:  EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
> 
> And this implementation layers on top of a TimerLib
> 
> armvirtqemu.dsc:  
> TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> 
> There are a couple PCDs involved in this module and lib. Maybe the 
> ArmVirtPkg needs to set some different PCD values to get a more 
> accurate
> gBS->Stall() when running through QEMU.

The issue is different; I checked gBS->Stall() in the above context and it waits for the appropriate time (usually just 1-2 microseconds more than requested).

Instead, the following happens:

- TerminalConInTimerHandler() in
"MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c" calls
SerialIo->GetControl().

- If the GetControl() call fails (for any reason), or the returned Control word has EFI_SERIAL_INPUT_BUFFER_EMPTY clear, then
TerminalConInTimerHandler() enters a loop:

    //
    // Fetch all the keys in the serial buffer,
    // and insert the byte stream into RawFIFO.
    //

- The loop body calls GetOneKeyFromSerial() --> SerialIo->Read(), with a
1 byte buffer.

- The loop runs until the "raw data FIFO buffer" is filled completely
(256 byte is the size, apparently -- RAW_FIFO_MAX_NUMBER), or
GetOneKeyFromSerial() returns an error.

- The SerialPortGetControl() function in "ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c"
returns constant RETURN_UNSUPPORTED. According to the library class header "MdePkg/Include/Library/SerialPortLib.h", this is a valid thing to do ("The serial device does not support this operation"). However, it will cause TerminalConInTimerHandler() to *always* enter the loop, even if there is no pending data to read.

- Because the input queue is empty, GetOneKeyFromSerial() will take a full second before it times out.

- And the final piece of the puzzle is that the event associated with
TerminalConInTimerHandler() is signaled at 50Hz (0.02s period, see KEYBOARD_TIMER_INTERVAL), initially. It can dynamically adjust its frequency to the serial device's timeout, but in practice, as soon as the current TerminalConInTimerHandler() frame returns, there's
(apparently) another execution queued already. So basically we're stuck in the timer event handler.

I think we should implement the missing ("unsupported") functions in "ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c" and possibly in "ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c".
I believe we could use
"ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c" as an example.

Thanks
Laszlo

Re: [PATCH] MdeModulePkg SerialDxe: Process timeout consistently in SerialRead

[Laszlo Ersek]

Star,

On 08/16/17 04:22, Zeng, Star wrote:
>   *Control = 0;
>   if (!SerialPortPoll ()) {
>     *Control = EFI_SERIAL_INPUT_BUFFER_EMPTY;
>   }
>   return RETURN_SUCCESS;
>
> As example, the code above (in
> OvmfPkg\Library\XenConsoleSerialPortLib\XenConsoleSerialPortLib.c) can
> be simply used in SerialPortGetControl().

Thank you for jogging my memory about this. I found the following commit
in the git history:

  ad7f6bc2e116 ("ArmVirtPkg: Use SerialDxe in MdeModulePkg instead of
                EmbeddedPkg", 2015-11-26)

and then I found our earlier discussion from 2015:

  http://mid.mail-archive.com/1447752930-32880-12-git-send-email-star.zeng@intel.com
  http://mid.mail-archive.com/1448243067-1880-12-git-send-email-star.zeng@intel.com

Basically the discussion went like this:

(1) In your v1 patch, you disconnected the FdtPL011SerialPortLib
    instance from the SerialPortExtLib class, and implemented the new
    functions (GetControl(), SetControl(), SetAttributes()) immediately
    by calling PL011UartLib.

(2) I requested that first we just copy the existing code from the
    EmbeddedPkg/Library/SerialPortExtLibNull instance -- which
    FdtPL011SerialPortLib had used up to that point --, and call
    PL011UartLib in a separate patch (later, if at all).

(3) In your v3 patch, you chose an approach that was different from
    *both* copying SerialPortExtLibNull *and* calling PL011UartLib: the
    new functions would simply return RETURN_UNSUPPORTED.

(4) Analyzing the TerminalDxe driver at that point, we agreed:

    - that a retval check had to be added after the GetControl() call in
      TerminalDxe,

    - more importantly, that with the error check in place, it was
      *equivalent* for GetControl() to return RETURN_UNSUPPORTED -- seen
      in your v3 patch --, or to return RETURN_SUCCESS and set Control
      to zero -- seen in the original code in SerialPortExtLibNull.

      In both cases, we expected TerminalDxe to work correctly, given
      that GetOneKeyFromSerial() would exit the loop on the first
      iteration anyway. In other words, setting
      EFI_SERIAL_INPUT_BUFFER_EMPTY in Control (and returning success)
      would only be an unimportant optimization.

(5) We agreed that calling PL011UartLib from the new APIs could be a
    future feature addition.

And that's the status what we have right now.

Except, with the SerialDxe change in commit 4cf3f37c87ba ("MdeModulePkg
SerialDxe: Process timeout consistently in SerialRead", 2017-07-18),
setting EFI_SERIAL_INPUT_BUFFER_EMPTY in Control is no longer an
"unimportant optimization", because now GetOneKeyFromSerial() takes
*very long* to exit the loop on the first iteration.

So even the first iteration must be prevented if the input queue is
empty, and that requires implementing GetControl() for real.

I think I'll submit an FdtPL011SerialPortLib patch, similar to your v1
approach. PL011UartLib implements these functions; we should use them.

Thanks!
Laszlo

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, August 16, 2017 10:02 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star
> <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Heyi Guo <heyi.guo@linaro.org>;
> Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [PATCH] MdeModulePkg SerialDxe: Process timeout
> consistently in SerialRead
>
> On 08/16/17 01:59, Kinney, Michael D wrote:
>> Laszlo,
>>
>> gBS->Stall() layers on top of the Metronome Architectural Protocol.
>>
>> armvirtqemu.dsc:  EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
>>
>> And this implementation layers on top of a TimerLib
>>
>> armvirtqemu.dsc:
>> TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
>>
>> There are a couple PCDs involved in this module and lib. Maybe the
>> ArmVirtPkg needs to set some different PCD values to get a more
>> accurate
>> gBS->Stall() when running through QEMU.
>
> The issue is different; I checked gBS->Stall() in the above context
> and it waits for the appropriate time (usually just 1-2 microseconds
> more than requested).
>
> Instead, the following happens:
>
> - TerminalConInTimerHandler() in
> "MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c" calls
> SerialIo->GetControl().
>
> - If the GetControl() call fails (for any reason), or the returned
> Control word has EFI_SERIAL_INPUT_BUFFER_EMPTY clear, then
> TerminalConInTimerHandler() enters a loop:
>
>     //
>     // Fetch all the keys in the serial buffer,
>     // and insert the byte stream into RawFIFO.
>     //
>
> - The loop body calls GetOneKeyFromSerial() --> SerialIo->Read(), with
> a 1 byte buffer.
>
> - The loop runs until the "raw data FIFO buffer" is filled completely
> (256 byte is the size, apparently -- RAW_FIFO_MAX_NUMBER), or
> GetOneKeyFromSerial() returns an error.
>
> - The SerialPortGetControl() function in
> "ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c"
> returns constant RETURN_UNSUPPORTED. According to the library class
> header "MdePkg/Include/Library/SerialPortLib.h", this is a valid thing
> to do ("The serial device does not support this operation"). However,
> it will cause TerminalConInTimerHandler() to *always* enter the loop,
> even if there is no pending data to read.
>
> - Because the input queue is empty, GetOneKeyFromSerial() will take a
> full second before it times out.
>
> - And the final piece of the puzzle is that the event associated with
> TerminalConInTimerHandler() is signaled at 50Hz (0.02s period, see
> KEYBOARD_TIMER_INTERVAL), initially. It can dynamically adjust its
> frequency to the serial device's timeout, but in practice, as soon as
> the current TerminalConInTimerHandler() frame returns, there's
> (apparently) another execution queued already. So basically we're
> stuck in the timer event handler.
>
> I think we should implement the missing ("unsupported") functions in
> "ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c" and
> possibly in
> "ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c".
> I believe we could use
> "ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c" as an
> example.
>
> Thanks
> Laszlo
>