On 26 January 2016 at 03:34, Laszlo Ersek <[email protected]> wrote: > On 01/26/16 03:55, Ni, Ruiyu wrote: >> Please correct me if I was wrong: >> Do all these discussions mean the FIFO depth fix is not the root cause? > > After re-reading the below again -- I don't think so. > > The test results seem to be consistent as far as the cursor keys are > concerned. With the patches applied, they work; without the patches, > they break. > > So, the test results for the cursor keys have been pretty solid. >
I agree. My conclusion is that FIFO depth was the problem. Changing my platform to use TTYTERM by default simply allows key codes to be interpreted correctly for Linux users. > The tests are inconsistent with regard to F9, which appears to require > TTYTERM to be testable from a Linux xterm. Yes. I think this is simply a case of other term types expecting F9 as a different sequence than Linux terminals send. This also applies to backspace, where it only works with TTYTERM from a Linux terminal. With term types other than TTYTERM selected in EDK2, backspace acts like DEL, both in Shell and in Intel BDS. > However there seems to be > some interference from "ArmPlatformPkg/Library/PlatformIntelBdsLib" that > is supposed to connect PcdDefaultConInPaths and PcdDefaultConOutPaths. > In order to experiment with F9, Roy massaged the last node of the > terminal device path (for controlling the parsing of the escape > sequences). But, some of those cases proved untestable. > > Unfortunately, I can't really help with > "ArmPlatformPkg/Library/PlatformIntelBdsLib". ArmVirtPkg used that > library instance as well originally, but I practically rewrote it > separately for ArmVirtPkg, in SVN r16923 and r16924. Since then, the two > instances have lived separate lives. > Is this wise? It looks like BDS history repeating itself :-/ > Thanks > Laszlo > >> >> Regards, >> Ray >> >> >> -----Original Message----- >> From: Laszlo Ersek [mailto:[email protected]] >> Sent: Friday, January 22, 2016 3:56 AM >> To: Ryan Harkin <[email protected]> >> Cc: Ni, Ruiyu <[email protected]>; Leif Lindholm >> <[email protected]>; [email protected]; Zeng, Star >> <[email protected]>; Ard Biesheuvel <[email protected]>; Roy Franz >> <[email protected]> >> Subject: Re: [edk2] [PATCH 3/3] MdeModulePkg: TerminalDxe: select the UART's >> default receive FIFO depth >> >> On 01/21/16 20:47, Ryan Harkin wrote: >>> On 21 January 2016 at 18:14, Laszlo Ersek <[email protected]> wrote: >>>> On 01/21/16 18:57, Ryan Harkin wrote: >>>>> On 21 January 2016 at 17:19, Laszlo Ersek <[email protected]> wrote: >>>> >>>>>> Ryan, can you please test the following configuration: >>>>>> - your PCD update in place, but this three-part series of mine >>>>>> *reverted* >>>>>> >>>>>> This should break the cursor movement keys again, and also break the >>>>>> function keys, despite the escape sequences being correct (thanks to >>>>>> your PCD update). This would answer Ray's question. >>>>>> >>>>> >>>>> I'm happy to give this a go. But I'm having trouble returning to a >>>>> setup where the cursor keys are broken at all. I've reverted the PCD >>>>> setting and F9 no longer works, but ... I don't know what I'm doing >>>>> wrong. I think I need to take a break from it for 1/2 hour and come >>>>> back to work it out. I'm pretty sure I've cleaned the repo and >>>>> flushed the builds... it's weird. >>>>> >>>>> I'll report back soon. >>>> >>>> Right, I proposed the opposite config for testing -- *keep* the PCD >>>> changes, and revert the series in your local tree whose thread we're >>>> conducting this discussion in. :) >>>> >>> >>> I wasn't clear - I reverted your three patches: >>> >>>> #1: revert 31ae446 MdeModulePkg: TerminalDxe: select the UART's default >>>> receive FIFO depth >>>> #2: revert ea01261 MdeModulePkg: SerialDxe: sync >>>> EFI_SERIAL_IO_MODE.Timeout with the spec >>>> #3: revert c0beed6 MdeModulePkg: SerialDxe: lay out mSerialIoMode >>>> initializer more nicely >>>> >>> >>> ... and cursor keys still worked on the serial port. So I reverted >>> the PCD changes too. And cursor keys kept on working. >>> >>> Now that I've worked out what was happening, let me first do your test >>> before I explain why the X Files came to visit. >>> >>> Using pure upstream EDK2 code: >>> >>> 1) No SerialDxe patches and no PCD fix >>> This means no cursor keys. >>> >>> 2) Using the PCD fix only >>> >>> I made this change: >>> >>> - >>> gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenPcAnsi();VenHw(407B4008-BF5B-11DF-9547-CF16E0D72085)" >>> - >>> gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(38400,8,N,1)/VenPcAnsi()" >>> + >>> gArmPlatformTokenSpaceGuid.PcdDefaultConOutPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(115200,8,N,1)/VenMsg(7D916D80-5BB1-458C-A48F-E25FDD51EF94)"^M >>> + >>> gArmPlatformTokenSpaceGuid.PcdDefaultConInPaths|L"VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(115200,8,N,1)/VenMsg(7D916D80-5BB1-458C-A48F-E25FDD51EF94)"^M >>> >>> And it fails to boot, giving this error message: >>> >>> Fail to start the console with the Device Path >>> 'VenHw(D3987D4B-971A-435F-8CAF-4967EB627241)/Uart(115200,8,N,1)/VenMsg(7D916D80-5BB1-458C-A48F-E25FDD51EF94)'. >>> (Error 'Not Found') >>> >>> I guess there's something missing in the upstream .DSC file :-( >>> >>> So I went back to using OpenPlatformPkg - but without building Shell >>> from source. >>> >>> And it doesn't work. >>> >>> 3) I add back in your 3 SerialDxe patches >>> The serial port comes back to life. >>> >>> >>> >>> >>>> (Although I find it somewhat flattering that I apparently fixed the cursor >>>> keys so well that they *cannot* be broken, even if you try! ;)) >>> >>> :-) >>> >>> So I've worked out what my anomaly was. >>> >>> It turns out that having this patch in my tree also fixes cursor keys >>> on the serial port, but not function keys like F9. No, I have no clue >>> what this patch has to do with the serial port, other than I am >>> testing the cursor keys AFTER Shell has run, so the latest version of >>> Shell could be doing something funky with the serial port setup. Like >>> changing the FIFO length. >>> >>> commit eaa23c12db4d32879bb4a6f826d7e003936e11df >>> Author: Ryan Harkin <[email protected]> >>> Date: Thu Jan 21 08:32:11 2016 +0000 >>> >>> Platforms/ARM: fvp: build Shell from source >>> >>> Rather than use a pre-built Shell binary, build the Shell from source. >>> >>> This allows the developer to debug and fix bugs in Shell, as well as >>> test the latest source code. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Ryan Harkin <[email protected]> >>> --- >>> Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf | 2 +- >>> Platforms/ARM/VExpress/ArmVExpress.dsc.inc | 19 +++++++++++++++++++ >>> 2 files changed, 20 insertions(+), 1 deletion(-) >>> >>> diff --git a/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf >>> b/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf >>> index e3ff0cc..a1ae35a 100644 >>> --- a/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf >>> +++ b/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf >>> @@ -214,7 +214,7 @@ FvNameGuid = >>> 87940482-fc81-41c3-87e6-399cf85ac8a0 >>> # >>> # UEFI application (Shell Embedded Boot Loader) >>> # >>> - INF ShellBinPkg/UefiShell/UefiShell.inf >>> + INF ShellPkg/Application/Shell/Shell.inf^M >>> >>> # >>> # Bds >>> diff --git a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc >>> b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc >>> index 86bad01..d6b02b9 100644 >>> --- a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc >>> +++ b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc >>> @@ -483,3 +483,22 @@ >>> >>> # Legacy Linux Loader >>> ArmPkg/Application/LinuxLoader/LinuxLoader.inf >>> +^M >>> + #^M >>> + # UEFI application (Shell Embedded Boot Loader)^M >>> + #^M >>> + ShellPkg/Application/Shell/Shell.inf {^M >>> + <LibraryClasses>^M >>> + >>> ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf^M >>> + >>> NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf^M >>> + >>> NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf^M >>> + >>> NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf^M >>> + >>> NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf^M >>> + >>> NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf^M >>> + >>> NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf^M >>> + >>> NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf^M >>> + >>> NULL|ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.inf^M >>> + >>> HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf^M >>> + PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf^M >>> + >>> BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf^M >>> + }^M >>> >>> >>> Now I need a lie down after all that madness :-O >>> >> >> Ugh. This leaves me confused. I think I'll have to read it very slowly >> one more time at least. :) >> >> Thanks >> Laszlo >> > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
