Hi,
The TimeOut handling in SerialRead() in SerialDxe(MdeModulepkg),
IsaSerialRead() in IsaSerialDxe(IntelFrameworkModulePkg) and SerialRead() in
PciSioSerialDxe(MdeModulePkg) are consistent, and we did not see this kind of
"slow down" before.
After some investigation, I found it is related to the Timeout value.
The Timeout is 1000000 (1s) by default to follow UEFI spec. And the Terminal
driver will recalculate and set the Timeout value based on the properties of
UART in TerminalDriverBindingStart()/TerminalConInTimerHandler().
SerialInTimeOut = 0;
if (Mode->BaudRate != 0) {
//
// According to BAUD rate to calculate the timeout value.
//
SerialInTimeOut = (1 + Mode->DataBits + Mode->StopBits) * 2 * 1000000 /
(UINTN) Mode->BaudRate;
}
For example, based on the PCD values of PcdUartDefaultBaudRate,
PcdUartDefaultDataBits and PcdUartDefaultStopBits, SerialInTimeOut = (1 + 8 +
1) * 2 * 1000000 / (UINTN) 115200 = 173 (us).
When SerialDxe is used,
TerminalDriverBindingStart()/TerminalConInTimerHandler() ->
SerialIo->SetAttributes() ->
SerialSetAttributes() ->
SerialPortSetAttributes()
Some implementations of SerialPortSetAttributes() could handle the input
parameters and return RETURN_SUCCESS, for example BaseSerialPortLib16550, then
Timeout value will be changed to 173 (us), no "slow down" will be observed.
But some implementations of SerialPortSetAttributes() just return
RETURN_UNSUPPORTED, for example XenConsoleSerialPortLib, then Timeout value
will be not changed and kept to be 1000000 (1s), "slow down" will be observed.
Here, how about to?
1. Handle the input parameters and return status accordingly instead of just
returning RETURN_UNSUPPORTED in SerialPortSetAttributes().
2. Just return RETURN_SUCCESS instead of RETURN_UNSUPPORTED in
SerialPortSetAttributes() if the instance does not care the input parameters at
all.
And SerialDxe may can be enhanced like below to be more robust.
==========
6ec9c40f91fc675ee77f3e54aea4e5a41a2de504
MdeModulePkg/Universal/SerialDxe/SerialIo.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
index ebcd92726314..060ea56c2b1a 100644
--- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
+++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
@@ -285,7 +285,21 @@ SerialSetAttributes (
Status = SerialPortSetAttributes (&BaudRate, &ReceiveFifoDepth, &Timeout,
&Parity, &DataBits, &StopBits);
if (EFI_ERROR (Status)) {
- return Status;
+ //
+ // If it is just to set Timeout value and unsupported is returned,
+ // do not return error.
+ //
+ if ((Status == EFI_UNSUPPORTED) &&
+ (This->Mode->Timeout != Timeout) &&
+ (This->Mode->ReceiveFifoDepth == ReceiveFifoDepth) &&
+ (This->Mode->BaudRate == BaudRate) &&
+ (This->Mode->DataBits == (UINT32) DataBits) &&
+ (This->Mode->Parity == (UINT32) Parity) &&
+ (This->Mode->StopBits == (UINT32) StopBits)) {
+ Status = EFI_SUCCESS;
+ } else {
+ return Status;
+ }
}
//
====================
Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:[email protected]] On Behalf Of Julien
Grall
Sent: Friday, October 27, 2017 2:32 AM
To: Laszlo Ersek <[email protected]>; edk2-devel-01 <[email protected]>;
Zeng, Star <[email protected]>; [email protected]; Ni, Ruiyu
<[email protected]>
Cc: Anthony PERARD <[email protected]>; Leif Lindholm
<[email protected]>; Ard Biesheuvel <[email protected]>
Subject: Re: [edk2] Xen Console input very slow in recent UEFI
Hi Laszlo,
Thank you for your help.
On 26/10/17 16:20, Laszlo Ersek wrote:
> On 10/26/17 17:13, Laszlo Ersek wrote:
>> Hello Julien,
>>
>> On 10/26/17 13:05, Julien Grall wrote:
>>> Hi all,
>>>
>>> I was doing more testing of UEFI in Xen guests and noticed some slow
>>> down when using the shell. The characters are only echoed after a
>>> second or two that is a bit annoying.
>>>
>>> The change that introduced this issue is 4cf3f37c87 "MdeModulePkg
>>> SerialDxe: Process timeout consistently in SerialRead".
>>>
>>> The Serial Driver for Xen PV console is very simple (see
>>> OvmfPkg/Library/XenConsoleSerialPortLib). So I am not sure where the
>>> root cause is.
>>>
>>> Would anyone have any tips on it?
>>
>> The exact same issue has been encountered earlier under QEMU, please
>> refer to the following sub-thread (please read it to end):
>>
>> http://mid.mail-archive.com/b748580c-cb51-32c9-acf9-780841ef15da@redh
>> at.com
>>
>> The fix was commit 5f0f5e90ae8c ("ArmVirtPkg/FdtPL011SerialPortLib:
>> call PL011UartLib in all SerialPortLib APIs", 2017-08-16).
>>
>> I think if you can implement the same for XenConsoleSerialPortLib,
>> that should return to working state as well.
>
> Hmmm, wait, at a closer look, it looks like
>
> OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c
>
> already implements that suggestion? (I.e., it sets
> EFI_SERIAL_INPUT_BUFFER_EMPTY in *Control as necessary?)
>
> Are we sure the SerialPortPoll() function works correctly? I don't see
> any MemoryFence() calls in SerialPortPoll(), around checking the
> fields in (*mXenConsoleInterface). Could that be the problem?
I am not entirely sure. But I added a couple of MemoryFence() in SerialPort
just in case to clear that from potential cause:
XENCONS_RING_IDX Consumer, Producer;
if (!mXenConsoleInterface) {
return FALSE;
}
MemoryFence ();
Consumer = mXenConsoleInterface->in_cons; Producer =
mXenConsoleInterface->in_prod;
MemoryFence ();
return Consumer != Producer;
I also added some debug printk (using a different interface) to confirm the
value of Consumer and Producer are valid. I can see the Producer increasing
every time a key is pressed and then soon followed by SerialPortRead
incrementing Consumer.
I did more debugging and find out the following is happening in
TerminalConInTimerHandler (MdeModulePkg/Universal/Console/TerminalDxe)
when a character is received:
1) GetControl will return EFI_SERIAL_INPUT_BUFFER_EMPTY unset
=> Entering in the loop to fetch character from the serial
2) GetOneKeyFromSerial()
=> Return directly with the character read
3) Looping as the fifo is not full and no error
4) GetOneKeyFromSerial() -> SerialRead()
=> No more character so SerialPortPoll() will return FALSE and loop
until timeout
=> Return EFI_TIMEOUT
5) Exiting the loop from TerminalConInTimerHandler
6) Characters are printed
So the step 4) will introduce the timeout seen and delay the echoing of the
characters received.
I could see a couple of solutions to fix it:
1) Remove the timeout from SerialPortRead and rely on either
a) caller to handle timeout
b) each UART driver
2) TerminalConInTimerHandler to check at every iteration whether the
buffer is empty.
Any other ideas?
Cheers,
--
Julien Grall
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
