From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.115; helo=mga14.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (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 06BF22034A875 for ; Thu, 26 Oct 2017 20:16:24 -0700 (PDT) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Oct 2017 20:20:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,303,1505804400"; d="scan'208";a="1235899167" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga002.fm.intel.com with ESMTP; 26 Oct 2017 20:20:11 -0700 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 26 Oct 2017 20:20:10 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 26 Oct 2017 20:20:10 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.159]) with mapi id 14.03.0319.002; Fri, 27 Oct 2017 11:20:08 +0800 From: "Zeng, Star" To: 'Julien Grall' , Laszlo Ersek , edk2-devel-01 , "heyi.guo@linaro.org" , "Ni, Ruiyu" CC: Anthony PERARD , Leif Lindholm , Ard Biesheuvel , "Zeng, Star" Thread-Topic: [edk2] Xen Console input very slow in recent UEFI Thread-Index: AQHTTkpxA/6A8SUWxES0Eju2m2RUQqL1tzkAgAACEQCAADVdAIABCqIg Date: Fri, 27 Oct 2017 03:20:08 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B9AEB7F@shsmsx102.ccr.corp.intel.com> References: <7516a301-dfbb-5bd4-5e1e-ccd18c26db30@linaro.org> <74e0ac36-195f-9275-922e-e499a513569f@redhat.com> <32b3c432-7f17-c387-5334-a6826d6fa769@redhat.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZGMzMjljOTMtODRhNC00Y2NlLWFlNWMtYzU4NWZiNjZmNDQwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJVd2ZRekNvc21OSVYrZ3NCXC9MSkhMSDhDb2JxVTFOQXlSXC9UZkd4XC9lQWx3c1JCdTFCYnZSMVVYUHZram55Z0ZoIn0= x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: Xen Console input very slow in recent UEFI 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: Fri, 27 Oct 2017 03:16:25 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi, The TimeOut handling in SerialRead() in SerialDxe(MdeModulepkg), IsaSerialR= ead() in IsaSerialDxe(IntelFrameworkModulePkg) and SerialRead() in PciSioS= erialDxe(MdeModulePkg) are consistent, and we did not see this kind of "slo= w 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 Termina= l driver will recalculate and set the Timeout value based on the properties= of UART in TerminalDriverBindingStart()/TerminalConInTimerHandler(). SerialInTimeOut =3D 0; if (Mode->BaudRate !=3D 0) { // // According to BAUD rate to calculate the timeout value. // SerialInTimeOut =3D (1 + Mode->DataBits + Mode->StopBits) * 2 * 10000= 00 / (UINTN) Mode->BaudRate; } For example, based on the PCD values of PcdUartDefaultBaudRate, PcdUartDefa= ultDataBits and PcdUartDefaultStopBits, SerialInTimeOut =3D (1 + 8 + 1) * = 2 * 1000000 / (UINTN) 115200 =3D 173 (us). When SerialDxe is used, TerminalDriverBindingStart()/TerminalConInTimerHandler() -> SerialIo->SetAttributes() -> SerialSetAttributes() -> SerialPortSetAttributes() Some implementations of SerialPortSetAttributes() could handle the input pa= rameters and return RETURN_SUCCESS, for example BaseSerialPortLib16550, the= n Timeout value will be changed to 173 (us), no "slow down" will be observe= d. But some implementations of SerialPortSetAttributes() just return RETURN_UN= SUPPORTED, 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 jus= t returning RETURN_UNSUPPORTED in SerialPortSetAttributes(). 2. Just return RETURN_SUCCESS instead of RETURN_UNSUPPORTED in SerialPortSe= tAttributes() if the instance does not care the input parameters at all. And SerialDxe may can be enhanced like below to be more robust. =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D 6ec9c40f91fc675ee77f3e54aea4e5a41a2de504 MdeModulePkg/Universal/SerialDxe/SerialIo.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c b/MdeModulePkg/Uni= versal/SerialDxe/SerialIo.c index ebcd92726314..060ea56c2b1a 100644 --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c @@ -285,7 +285,21 @@ SerialSetAttributes ( =20 Status =3D SerialPortSetAttributes (&BaudRate, &ReceiveFifoDepth, &Timeo= ut, &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 =3D=3D EFI_UNSUPPORTED) && + (This->Mode->Timeout !=3D Timeout) && + (This->Mode->ReceiveFifoDepth =3D=3D ReceiveFifoDepth) && + (This->Mode->BaudRate =3D=3D BaudRate) && + (This->Mode->DataBits =3D=3D (UINT32) DataBits) && + (This->Mode->Parity =3D=3D (UINT32) Parity) && + (This->Mode->StopBits =3D=3D (UINT32) StopBits)) { + Status =3D EFI_SUCCESS; + } else { + return Status; + } } =20 // =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D Thanks, Star -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Juli= en Grall Sent: Friday, October 27, 2017 2:32 AM To: Laszlo Ersek ; edk2-devel-01 ; Zeng, Star ; heyi.guo@linaro.org; Ni, Ruiyu Cc: Anthony PERARD ; Leif Lindholm ; Ard Biesheuvel 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=20 >>> down when using the shell. The characters are only echoed after a=20 >>> 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=20 >>> OvmfPkg/Library/XenConsoleSerialPortLib). So I am not sure where the=20 >>> root cause is. >>> >>> Would anyone have any tips on it? >> >> The exact same issue has been encountered earlier under QEMU, please=20 >> 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:=20 >> call PL011UartLib in all SerialPortLib APIs", 2017-08-16). >> >> I think if you can implement the same for XenConsoleSerialPortLib,=20 >> that should return to working state as well. >=20 > Hmmm, wait, at a closer look, it looks like >=20 > OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c >=20 > already implements that suggestion? (I.e., it sets=20 > EFI_SERIAL_INPUT_BUFFER_EMPTY in *Control as necessary?) >=20 > Are we sure the SerialPortPoll() function works correctly? I don't see=20 > any MemoryFence() calls in SerialPortPoll(), around checking the=20 > 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 =3D mXenConsoleInterface->in_cons; Producer =3D mXenConsoleInterfa= ce->in_prod; MemoryFence (); return Consumer !=3D 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 increasi= ng every time a key is pressed and then soon followed by SerialPortRead inc= rementing Consumer. I did more debugging and find out the following is happening in TerminalCon= InTimerHandler (MdeModulePkg/Universal/Console/TerminalDxe) when a character is received: 1) GetControl will return EFI_SERIAL_INPUT_BUFFER_EMPTY unset =3D> Entering in the loop to fetch character from the serial 2) GetOneKeyFromSerial() =3D> Return directly with the character read 3) Looping as the fifo is not full and no error 4) GetOneKeyFromSerial() -> SerialRead() =3D> No more character so SerialPortPoll() will return FALSE and loop u= ntil timeout =3D> 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 buffe= r is empty. Any other ideas? Cheers, -- Julien Grall _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel