Discussion:
[edk2] [PATCH V2 0/4] Add TtyTerm terminal type
Roy Franz
2015-07-06 23:04:35 UTC
Permalink
This patchset adds a new terminal type "TtyTerm", that better handles the
typical Linux terminal (xterm/rxvt/gnome terminal/tmux/screen/etc.) These often
treat backspace/delete differently than the existing EDK2 terminal types
expect, particularly in emulated environments where the emulated serial port is
connected to a graphical terminal rather than a serial console. Some terminals
use a mix of vt100 and vt200 escape codes for function key handling. Adding
vt220 function key support fixes the use of "F10" to save changes in BDS
configuration menus.

This patchset includes changes to make the terminal type build time configurable
for the QEMU Aarch64 configuration. Once this patchset goes in I will submit
patches for the other emulated ARM platforms as appropriate.

Note: While working on the function keys, I could not find any documentation
that matched the code/comments in TerminalConIn.c regarding VT100 function
keys. The VT100 only had 4 function keys (PF1-PF4), which are generally mapped
to F1-F4. In the code, F3/F4 don't match the VT100 documentation I found, and
I have no idea where the values for F5-F10 are from. I left the existing
VT100 terminal type escape code handling unchanged.


Changes since v1:
* Added handling of VT220 escape codes for function keys
* Fixed attribution PCD patch
* Removed '[' from patch 3 description line

Changes from Linuxterm RFC patchset:
* Change to ttyTerm name - nothing linux specific in patchset
* remove changes to Mde module, as changes not part of UEFI specification


Laszlo Ersek (1):
Add PCD for selecting terminal type at build time

Roy Franz (3):
Add "TtyTerm" terminal type to TerminalDxe
Treat ASCII 0x7F as backspace for TtyTerm terminals
Accept VT220 DEL and function keys for TTY terminal type

ArmVirtPkg/ArmVirt.dsc.inc | 6 ++
ArmVirtPkg/ArmVirtPkg.dec | 7 ++
ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c | 12 ++-
ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf | 4 +
MdeModulePkg/Include/Guid/TtyTerm.h | 25 +++++
MdeModulePkg/MdeModulePkg.dec | 3 +
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c | 44 ++++++--
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 3 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 109 +++++++++++++++++++-
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c | 2 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf | 1 +
11 files changed, 201 insertions(+), 15 deletions(-)
create mode 100644 MdeModulePkg/Include/Guid/TtyTerm.h
--
2.1.4
Roy Franz
2015-07-06 23:04:36 UTC
Permalink
This patch a adds new terminal type, TtyTerm, to TerminalDxe. This terminal
type provides a place to add support for various *nix terminals that don't
behave like standard VT terminals. The goal is to 'just work' with as many
terminals as possible, rather than properly emulating any one specific
terminal.

Signed-off-by: Roy Franz <***@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
---
MdeModulePkg/Include/Guid/TtyTerm.h | 25 +++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 ++
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c | 44 ++++++++++++++++----
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 2 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 4 +-
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c | 2 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf | 1 +
7 files changed, 73 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Include/Guid/TtyTerm.h b/MdeModulePkg/Include/Guid/TtyTerm.h
new file mode 100644
index 0000000..900e5af
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/TtyTerm.h
@@ -0,0 +1,25 @@
+/** @file
+GUID definition for TtyTerm terminal type. The TtyTerm terminal aims to
+provide support for modern *nix terminals.
+
+
+Copyright (c) 2015 Linaro Ltd.
+This program and the accompanying materials are licensed and made
+available under the terms and conditions of the BSD License that
+accompanies this distribution. The full text of the license may be found
+at http://opensource.org/licenses/bsd-license.php.
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __TTYTERM_H__
+#define __TTYTERM_H__
+
+#define EFI_TTY_TERM_GUID \
+ {0x7d916d80, 0x5bb1, 0x458c, {0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94 } }
+
+extern EFI_GUID gEfiTtyTermGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 414b13e..539463b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -304,6 +304,9 @@
# Include/Guid/LzmaDecompress.h
gLzmaCustomDecompressGuid = { 0xEE4E5898, 0x3914, 0x4259, { 0x9D, 0x6E, 0xDC, 0x7B, 0xD7, 0x94, 0x03, 0xCF }}
gLzmaF86CustomDecompressGuid = { 0xD42AE6BD, 0x1352, 0x4bfb, { 0x90, 0x9A, 0xCA, 0x72, 0xA6, 0xEA, 0xE8, 0x89 }}
+
+ ## Include/Guid/Guid/TtyTerm.h
+ gEfiTtyTermGuid = { 0x7d916d80, 0x5bb1, 0x458c, {0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94 }}

[Ppis]
## Include/Ppi/AtaController.h
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
index 966fb79..babb097 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
@@ -33,7 +33,8 @@ EFI_GUID *gTerminalType[] = {
&gEfiPcAnsiGuid,
&gEfiVT100Guid,
&gEfiVT100PlusGuid,
- &gEfiVTUTF8Guid
+ &gEfiVTUTF8Guid,
+ &gEfiTtyTermGuid
};


@@ -152,12 +153,13 @@ TerminalDriverBindingSupported (

}
//
- // only supports PC ANSI, VT100, VT100+ and VT-UTF8 terminal types
+ // only supports PC ANSI, VT100, VT100+, VT-UTF8, and TtyTerm terminal types
//
if (!CompareGuid (&Node->Guid, &gEfiPcAnsiGuid) &&
!CompareGuid (&Node->Guid, &gEfiVT100Guid) &&
!CompareGuid (&Node->Guid, &gEfiVT100PlusGuid) &&
- !CompareGuid (&Node->Guid, &gEfiVTUTF8Guid)) {
+ !CompareGuid (&Node->Guid, &gEfiVTUTF8Guid) &&
+ !CompareGuid (&Node->Guid, &gEfiTtyTermGuid)) {

return EFI_UNSUPPORTED;
}
@@ -275,6 +277,10 @@ BuildTerminalDevpath (

TerminalType = VTUTF8TYPE;

+ } else if (CompareGuid (&Node->Guid, &gEfiTtyTermGuid)) {
+
+ TerminalType = TTYTERMTYPE;
+
} else {
return NULL;
}
@@ -708,9 +714,9 @@ TerminalDriverBindingStart (

TerminalType = PcdGet8 (PcdDefaultTerminalType);
//
- // Must be between PCANSITYPE (0) and VTUTF8TYPE (3)
+ // Must be between PCANSITYPE (0) and TTYTERMTYPE (4)
//
- ASSERT (TerminalType <= VTUTF8TYPE);
+ ASSERT (TerminalType <= TTYTERMTYPE);

CopyMem (&DefaultNode->Guid, gTerminalType[TerminalType], sizeof (EFI_GUID));
RemainingDevicePath = (EFI_DEVICE_PATH_PROTOCOL *) DefaultNode;
@@ -728,6 +734,8 @@ TerminalDriverBindingStart (
TerminalType = VT100PLUSTYPE;
} else if (CompareGuid (&Node->Guid, &gEfiVTUTF8Guid)) {
TerminalType = VTUTF8TYPE;
+ } else if (CompareGuid (&Node->Guid, &gEfiTtyTermGuid)) {
+ TerminalType = TTYTERMTYPE;
} else {
goto Error;
}
@@ -926,6 +934,24 @@ TerminalDriverBindingStart (
);

break;
+
+ case TTYTERMTYPE:
+ AddUnicodeString2 (
+ "eng",
+ gTerminalComponentName.SupportedLanguages,
+ &TerminalDevice->ControllerNameTable,
+ (CHAR16 *)L"Tty Terminal Serial Console",
+ TRUE
+ );
+ AddUnicodeString2 (
+ "en",
+ gTerminalComponentName2.SupportedLanguages,
+ &TerminalDevice->ControllerNameTable,
+ (CHAR16 *)L"Tty Terminal Serial Console",
+ FALSE
+ );
+
+ break;
}

//
@@ -1441,7 +1467,7 @@ TerminalUpdateConsoleDevVariable (
//
// Append terminal device path onto the variable.
//
- for (TerminalType = PCANSITYPE; TerminalType <= VTUTF8TYPE; TerminalType++) {
+ for (TerminalType = PCANSITYPE; TerminalType <= TTYTERMTYPE; TerminalType++) {
SetTerminalDevicePath (TerminalType, ParentDevicePath, &TempDevicePath);
NewVariable = AppendDevicePathInstance (Variable, TempDevicePath);
ASSERT (NewVariable != NULL);
@@ -1554,7 +1580,7 @@ TerminalRemoveConsoleDevVariable (
// Loop through all the terminal types that this driver supports
//
Match = FALSE;
- for (TerminalType = PCANSITYPE; TerminalType <= VTUTF8TYPE; TerminalType++) {
+ for (TerminalType = PCANSITYPE; TerminalType <= TTYTERMTYPE; TerminalType++) {

SetTerminalDevicePath (TerminalType, ParentDevicePath, &TempDevicePath);

@@ -1658,6 +1684,10 @@ SetTerminalDevicePath (
CopyGuid (&Node.Guid, &gEfiVTUTF8Guid);
break;

+ case TTYTERMTYPE:
+ CopyGuid (&Node.Guid, &gEfiTtyTermGuid);
+ break;
+
default:
return EFI_UNSUPPORTED;
}
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index d393acb..03542a4 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -20,6 +20,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

#include <Guid/GlobalVariable.h>
#include <Guid/PcAnsi.h>
+#include <Guid/TtyTerm.h>
#include <Guid/StatusCodeDataTypeVariable.h>

#include <Protocol/SimpleTextOut.h>
@@ -136,6 +137,7 @@ typedef union {
#define VT100TYPE 1
#define VT100PLUSTYPE 2
#define VTUTF8TYPE 3
+#define TTYTERMTYPE 4

#define LEFTOPENBRACKET 0x5b // '['
#define ACAP 0x41
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 4a008c9..17a1244 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -454,6 +454,7 @@ TranslateRawDataToEfiKey (
case PCANSITYPE:
case VT100TYPE:
case VT100PLUSTYPE:
+ case TTYTERMTYPE:
AnsiRawDataToUnicode (TerminalDevice);
UnicodeToEfiKey (TerminalDevice);
break;
@@ -1393,7 +1394,8 @@ UnicodeToEfiKey (
if (TerminalDevice->TerminalType == PCANSITYPE ||
TerminalDevice->TerminalType == VT100TYPE ||
TerminalDevice->TerminalType == VT100PLUSTYPE ||
- TerminalDevice->TerminalType == VTUTF8TYPE) {
+ TerminalDevice->TerminalType == VTUTF8TYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE) {
switch (UnicodeChar) {
case 'A':
Key.ScanCode = SCAN_UP;
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
index affb3ae..9fa952a 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
@@ -223,6 +223,7 @@ TerminalConOutOutputString (
case PCANSITYPE:
case VT100TYPE:
case VT100PLUSTYPE:
+ case TTYTERMTYPE:

if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar)) {
//
@@ -371,6 +372,7 @@ TerminalConOutTestString (
case PCANSITYPE:
case VT100TYPE:
case VT100PLUSTYPE:
+ case TTYTERMTYPE:
Status = AnsiTestString (TerminalDevice, WString);
break;

diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
index 1d86388..0780296 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
@@ -73,6 +73,7 @@
gEfiVT100Guid ## SOMETIMES_CONSUMES ## GUID # used with a Vendor-Defined Messaging Device Path
gEfiVT100PlusGuid ## SOMETIMES_CONSUMES ## GUID # used with a Vendor-Defined Messaging Device Path
gEfiPcAnsiGuid ## SOMETIMES_CONSUMES ## GUID # used with a Vendor-Defined Messaging Device Path
+ gEfiTtyTermGuid ## SOMETIMES_CONSUMES ## GUID # used with a Vendor-Defined Messaging Device Path
gEdkiiStatusCodeDataTypeVariableGuid ## SOMETIMES_CONSUMES ## GUID

[Protocols]
--
2.1.4
Ard Biesheuvel
2015-07-07 13:08:58 UTC
Permalink
Post by Roy Franz
This patch a adds new terminal type, TtyTerm, to TerminalDxe. This terminal
type provides a place to add support for various *nix terminals that don't
behave like standard VT terminals. The goal is to 'just work' with as many
terminals as possible, rather than properly emulating any one specific
terminal.
Contributed-under: TianoCore Contribution Agreement 1.0
---
MdeModulePkg/Include/Guid/TtyTerm.h | 25 +++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 ++
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c | 44 ++++++++++++++++----
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 2 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 4 +-
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c | 2 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf | 1 +
7 files changed, 73 insertions(+), 8 deletions(-)
diff --git a/MdeModulePkg/Include/Guid/TtyTerm.h b/MdeModulePkg/Include/Guid/TtyTerm.h
new file mode 100644
index 0000000..900e5af
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/TtyTerm.h
@@ -0,0 +1,25 @@
+GUID definition for TtyTerm terminal type. The TtyTerm terminal aims to
+provide support for modern *nix terminals.
+
+
+Copyright (c) 2015 Linaro Ltd.
+This program and the accompanying materials are licensed and made
+available under the terms and conditions of the BSD License that
+accompanies this distribution. The full text of the license may be found
+at http://opensource.org/licenses/bsd-license.php.
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __TTYTERM_H__
+#define __TTYTERM_H__
+
+#define EFI_TTY_TERM_GUID \
+ {0x7d916d80, 0x5bb1, 0x458c, {0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94 } }
+
+extern EFI_GUID gEfiTtyTermGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 414b13e..539463b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -304,6 +304,9 @@
# Include/Guid/LzmaDecompress.h
gLzmaCustomDecompressGuid = { 0xEE4E5898, 0x3914, 0x4259, { 0x9D, 0x6E, 0xDC, 0x7B, 0xD7, 0x94, 0x03, 0xCF }}
gLzmaF86CustomDecompressGuid = { 0xD42AE6BD, 0x1352, 0x4bfb, { 0x90, 0x9A, 0xCA, 0x72, 0xA6, 0xEA, 0xE8, 0x89 }}
+
+ ## Include/Guid/Guid/TtyTerm.h
Typo here ^^^
Post by Roy Franz
+ gEfiTtyTermGuid = { 0x7d916d80, 0x5bb1, 0x458c, {0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94 }}
[Ppis]
## Include/Ppi/AtaController.h
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
index 966fb79..babb097 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
@@ -33,7 +33,8 @@ EFI_GUID *gTerminalType[] = {
&gEfiPcAnsiGuid,
&gEfiVT100Guid,
&gEfiVT100PlusGuid,
- &gEfiVTUTF8Guid
+ &gEfiVTUTF8Guid,
+ &gEfiTtyTermGuid
};
@@ -152,12 +153,13 @@ TerminalDriverBindingSupported (
}
//
- // only supports PC ANSI, VT100, VT100+ and VT-UTF8 terminal types
+ // only supports PC ANSI, VT100, VT100+, VT-UTF8, and TtyTerm terminal types
//
if (!CompareGuid (&Node->Guid, &gEfiPcAnsiGuid) &&
!CompareGuid (&Node->Guid, &gEfiVT100Guid) &&
!CompareGuid (&Node->Guid, &gEfiVT100PlusGuid) &&
- !CompareGuid (&Node->Guid, &gEfiVTUTF8Guid)) {
+ !CompareGuid (&Node->Guid, &gEfiVTUTF8Guid) &&
+ !CompareGuid (&Node->Guid, &gEfiTtyTermGuid)) {
return EFI_UNSUPPORTED;
}
@@ -275,6 +277,10 @@ BuildTerminalDevpath (
TerminalType = VTUTF8TYPE;
+ } else if (CompareGuid (&Node->Guid, &gEfiTtyTermGuid)) {
+
+ TerminalType = TTYTERMTYPE;
+
} else {
return NULL;
}
@@ -708,9 +714,9 @@ TerminalDriverBindingStart (
TerminalType = PcdGet8 (PcdDefaultTerminalType);
//
- // Must be between PCANSITYPE (0) and VTUTF8TYPE (3)
+ // Must be between PCANSITYPE (0) and TTYTERMTYPE (4)
//
- ASSERT (TerminalType <= VTUTF8TYPE);
+ ASSERT (TerminalType <= TTYTERMTYPE);
CopyMem (&DefaultNode->Guid, gTerminalType[TerminalType], sizeof (EFI_GUID));
RemainingDevicePath = (EFI_DEVICE_PATH_PROTOCOL *) DefaultNode;
@@ -728,6 +734,8 @@ TerminalDriverBindingStart (
TerminalType = VT100PLUSTYPE;
} else if (CompareGuid (&Node->Guid, &gEfiVTUTF8Guid)) {
TerminalType = VTUTF8TYPE;
+ } else if (CompareGuid (&Node->Guid, &gEfiTtyTermGuid)) {
+ TerminalType = TTYTERMTYPE;
} else {
goto Error;
}
@@ -926,6 +934,24 @@ TerminalDriverBindingStart (
);
break;
+
+ AddUnicodeString2 (
+ "eng",
+ gTerminalComponentName.SupportedLanguages,
+ &TerminalDevice->ControllerNameTable,
+ (CHAR16 *)L"Tty Terminal Serial Console",
+ TRUE
+ );
+ AddUnicodeString2 (
+ "en",
+ gTerminalComponentName2.SupportedLanguages,
+ &TerminalDevice->ControllerNameTable,
+ (CHAR16 *)L"Tty Terminal Serial Console",
+ FALSE
+ );
+
+ break;
}
//
@@ -1441,7 +1467,7 @@ TerminalUpdateConsoleDevVariable (
//
// Append terminal device path onto the variable.
//
- for (TerminalType = PCANSITYPE; TerminalType <= VTUTF8TYPE; TerminalType++) {
+ for (TerminalType = PCANSITYPE; TerminalType <= TTYTERMTYPE; TerminalType++) {
SetTerminalDevicePath (TerminalType, ParentDevicePath, &TempDevicePath);
NewVariable = AppendDevicePathInstance (Variable, TempDevicePath);
ASSERT (NewVariable != NULL);
@@ -1554,7 +1580,7 @@ TerminalRemoveConsoleDevVariable (
// Loop through all the terminal types that this driver supports
//
Match = FALSE;
- for (TerminalType = PCANSITYPE; TerminalType <= VTUTF8TYPE; TerminalType++) {
+ for (TerminalType = PCANSITYPE; TerminalType <= TTYTERMTYPE; TerminalType++) {
SetTerminalDevicePath (TerminalType, ParentDevicePath, &TempDevicePath);
@@ -1658,6 +1684,10 @@ SetTerminalDevicePath (
CopyGuid (&Node.Guid, &gEfiVTUTF8Guid);
break;
+ CopyGuid (&Node.Guid, &gEfiTtyTermGuid);
+ break;
+
return EFI_UNSUPPORTED;
}
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index d393acb..03542a4 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -20,6 +20,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Guid/GlobalVariable.h>
#include <Guid/PcAnsi.h>
+#include <Guid/TtyTerm.h>
#include <Guid/StatusCodeDataTypeVariable.h>
#include <Protocol/SimpleTextOut.h>
@@ -136,6 +137,7 @@ typedef union {
#define VT100TYPE 1
#define VT100PLUSTYPE 2
#define VTUTF8TYPE 3
+#define TTYTERMTYPE 4
#define LEFTOPENBRACKET 0x5b // '['
#define ACAP 0x41
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 4a008c9..17a1244 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -454,6 +454,7 @@ TranslateRawDataToEfiKey (
AnsiRawDataToUnicode (TerminalDevice);
UnicodeToEfiKey (TerminalDevice);
break;
@@ -1393,7 +1394,8 @@ UnicodeToEfiKey (
if (TerminalDevice->TerminalType == PCANSITYPE ||
TerminalDevice->TerminalType == VT100TYPE ||
TerminalDevice->TerminalType == VT100PLUSTYPE ||
- TerminalDevice->TerminalType == VTUTF8TYPE) {
+ TerminalDevice->TerminalType == VTUTF8TYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE) {
switch (UnicodeChar) {
Key.ScanCode = SCAN_UP;
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
index affb3ae..9fa952a 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
@@ -223,6 +223,7 @@ TerminalConOutOutputString (
if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar)) {
//
@@ -371,6 +372,7 @@ TerminalConOutTestString (
Status = AnsiTestString (TerminalDevice, WString);
break;
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
index 1d86388..0780296 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
@@ -73,6 +73,7 @@
gEfiVT100Guid ## SOMETIMES_CONSUMES ## GUID # used with a Vendor-Defined Messaging Device Path
gEfiVT100PlusGuid ## SOMETIMES_CONSUMES ## GUID # used with a Vendor-Defined Messaging Device Path
gEfiPcAnsiGuid ## SOMETIMES_CONSUMES ## GUID # used with a Vendor-Defined Messaging Device Path
+ gEfiTtyTermGuid ## SOMETIMES_CONSUMES ## GUID # used with a Vendor-Defined Messaging Device Path
gEdkiiStatusCodeDataTypeVariableGuid ## SOMETIMES_CONSUMES ## GUID
[Protocols]
--
2.1.4
Roy Franz
2015-07-06 23:04:37 UTC
Permalink
Treat ASCII 0x7F as backspace, rather than delete, for TTY terminals. This
better matches the default Linux terminal settings that are used when connecting
to a simulated platform using xterm or a similar terminal program.

Signed-off-by: Roy Franz <***@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
---
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 17a1244..227df85 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -1561,8 +1561,14 @@ UnicodeToEfiKey (
}

if (UnicodeChar == DEL) {
- Key.ScanCode = SCAN_DELETE;
- Key.UnicodeChar = 0;
+ if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+ Key.ScanCode = SCAN_NULL;
+ Key.UnicodeChar = CHAR_BACKSPACE;
+ }
+ else {
+ Key.ScanCode = SCAN_DELETE;
+ Key.UnicodeChar = 0;
+ }
} else {
Key.ScanCode = SCAN_NULL;
Key.UnicodeChar = UnicodeChar;
--
2.1.4
Ard Biesheuvel
2015-07-07 13:10:24 UTC
Permalink
Post by Roy Franz
Treat ASCII 0x7F as backspace, rather than delete, for TTY terminals. This
better matches the default Linux terminal settings that are used when connecting
to a simulated platform using xterm or a similar terminal program.
Contributed-under: TianoCore Contribution Agreement 1.0
Order is wrong ^^^

Other than that
Post by Roy Franz
---
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 17a1244..227df85 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -1561,8 +1561,14 @@ UnicodeToEfiKey (
}
if (UnicodeChar == DEL) {
- Key.ScanCode = SCAN_DELETE;
- Key.UnicodeChar = 0;
+ if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+ Key.ScanCode = SCAN_NULL;
+ Key.UnicodeChar = CHAR_BACKSPACE;
+ }
+ else {
+ Key.ScanCode = SCAN_DELETE;
+ Key.UnicodeChar = 0;
+ }
} else {
Key.ScanCode = SCAN_NULL;
Key.UnicodeChar = UnicodeChar;
--
2.1.4
Roy Franz
2015-07-06 23:04:38 UTC
Permalink
Accept the VT220 escape code [3~ as backspace for TtyTerm terminals. This is
sent by many Linux terminals by default. Also accept VT220 function keys
F1-F12, and VT100 F1-F4 keys as these are commonly sent by Linux terminals.
The VT220 escape codes are longer, and variable length so a new state is added
to the state machine along with a variable to construct the multibyte escape
sequence.
There are currently no ambiguous escape sequence prefixes accepted, so the TTY
terminal accepts escape sequences for a variety of terminals. The goal is to
'just work' with as many terminals as possible, rather than properly emulating
any specific terminal. Backspace, Del, and F10 have been tested on xterm,
rxvt, tmux, and screen.
Note: The existing vt100 function key handling does not match the vt100
documentation that I found, so I added the TTY terminal handling
of VT100 F1-F4 (really PF1-PF4 on vt100) separately. The vt100
has no F5-F10 keys, so I don't know what the current vt100 code
is based on.

Signed-off-by: Roy Franz <***@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
---
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 1 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 95 +++++++++++++++++++-
2 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 03542a4..4616ab3 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -118,6 +118,7 @@ typedef struct {
#define INPUT_STATE_LEFTOPENBRACKET 0x04
#define INPUT_STATE_O 0x08
#define INPUT_STATE_2 0x10
+#define INPUT_STATE_LEFTOPENBRACKET_2 0x20

#define RESET_STATE_DEFAULT 0x00
#define RESET_STATE_ESC_R 0x01
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 227df85..12e7f9f 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -1182,6 +1182,8 @@ UnicodeToEfiKey (
UINT16 UnicodeChar;
EFI_INPUT_KEY Key;
BOOLEAN SetDefaultResetState;
+ static UINT16 TtyEscapeStr[3];
+ static INTN TtyEscapeIndex;

TimerStatus = gBS->CheckEvent (TerminalDevice->TwoSecondTimeOut);

@@ -1223,7 +1225,8 @@ UnicodeToEfiKey (
continue;
}

- if (UnicodeChar == 'O' && TerminalDevice->TerminalType == VT100TYPE) {
+ if (UnicodeChar == 'O' && (TerminalDevice->TerminalType == VT100TYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE)) {
TerminalDevice->InputState |= INPUT_STATE_O;
TerminalDevice->ResetState = RESET_STATE_DEFAULT;
continue;
@@ -1371,6 +1374,22 @@ UnicodeToEfiKey (
default :
break;
}
+ } else if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+ /* Also accept VT100 escape codes for F1-F4 for TTY term */
+ switch (UnicodeChar) {
+ case 'P':
+ Key.ScanCode = SCAN_F1;
+ break;
+ case 'Q':
+ Key.ScanCode = SCAN_F2;
+ break;
+ case 'R':
+ Key.ScanCode = SCAN_F3;
+ break;
+ case 'S':
+ Key.ScanCode = SCAN_F4;
+ break;
+ }
}

if (Key.ScanCode != SCAN_NULL) {
@@ -1514,6 +1533,21 @@ UnicodeToEfiKey (
}
}

+ /*
+ * The VT220 escape codes that the TTY terminal accepts all have
+ * numeric codes, and there are no ambiguous prefixes shared with
+ * other terminal types.
+ */
+ if (TerminalDevice->TerminalType == TTYTERMTYPE &&
+ Key.ScanCode == SCAN_NULL &&
+ UnicodeChar >= '0' &&
+ UnicodeChar <= '9') {
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
+ TerminalDevice->InputState |= INPUT_STATE_LEFTOPENBRACKET_2;
+ continue;
+ }
+
if (Key.ScanCode != SCAN_NULL) {
Key.UnicodeChar = 0;
EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
@@ -1527,6 +1561,65 @@ UnicodeToEfiKey (
break;


+ case INPUT_STATE_ESC | INPUT_STATE_LEFTOPENBRACKET | INPUT_STATE_LEFTOPENBRACKET_2:
+ /*
+ * Here we handle the VT220 escape codes that we accept. This
+ * state is only used by the TTY terminal type.
+ */
+ Key.ScanCode = SCAN_NULL;
+ if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+
+ if (UnicodeChar == '~' && TtyEscapeIndex <= 2) {
+ UINTN EscCode;
+ TtyEscapeStr[TtyEscapeIndex] = 0; /* Terminate string */
+ EscCode = StrDecimalToUintn(TtyEscapeStr);
+ switch (EscCode) {
+ case 3:
+ Key.ScanCode = SCAN_DELETE;
+ break;
+ case 11:
+ case 12:
+ case 13:
+ case 14:
+ case 15:
+ Key.ScanCode = SCAN_F1 + EscCode - 11;
+ break;
+ case 17:
+ case 18:
+ case 19:
+ case 20:
+ case 21:
+ Key.ScanCode = SCAN_F6 + EscCode - 17;
+ break;
+ case 23:
+ case 24:
+ Key.ScanCode = SCAN_F11 + EscCode - 23;
+ break;
+ default:
+ break;
+ }
+ } else if (TtyEscapeIndex == 1){
+ /* 2 character escape code */
+ TtyEscapeStr[TtyEscapeIndex++] = UnicodeChar;
+ continue;
+ }
+ else {
+ DEBUG ((EFI_D_ERROR, "Unexpected state in escape2\n"));
+ }
+ }
+ TerminalDevice->ResetState = RESET_STATE_DEFAULT;
+
+ if (Key.ScanCode != SCAN_NULL) {
+ Key.UnicodeChar = 0;
+ EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
+ TerminalDevice->InputState = INPUT_STATE_DEFAULT;
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ continue;
+ }
+
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ break;
+
default:
//
// Invalid state. This should never happen.
--
2.1.4
Ard Biesheuvel
2015-07-07 13:18:30 UTC
Permalink
Post by Roy Franz
Accept the VT220 escape code [3~ as backspace for TtyTerm terminals. This is
sent by many Linux terminals by default. Also accept VT220 function keys
F1-F12, and VT100 F1-F4 keys as these are commonly sent by Linux terminals.
The VT220 escape codes are longer, and variable length so a new state is added
to the state machine along with a variable to construct the multibyte escape
sequence.
There are currently no ambiguous escape sequence prefixes accepted, so the TTY
terminal accepts escape sequences for a variety of terminals. The goal is to
'just work' with as many terminals as possible, rather than properly emulating
any specific terminal. Backspace, Del, and F10 have been tested on xterm,
rxvt, tmux, and screen.
Note: The existing vt100 function key handling does not match the vt100
documentation that I found, so I added the TTY terminal handling
of VT100 F1-F4 (really PF1-PF4 on vt100) separately. The vt100
has no F5-F10 keys, so I don't know what the current vt100 code
is based on.
Contributed-under: TianoCore Contribution Agreement 1.0
Same here ^^^
(and in patch 1/4)
Post by Roy Franz
---
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 1 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 95 +++++++++++++++++++-
2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 03542a4..4616ab3 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -118,6 +118,7 @@ typedef struct {
#define INPUT_STATE_LEFTOPENBRACKET 0x04
#define INPUT_STATE_O 0x08
#define INPUT_STATE_2 0x10
+#define INPUT_STATE_LEFTOPENBRACKET_2 0x20
#define RESET_STATE_DEFAULT 0x00
#define RESET_STATE_ESC_R 0x01
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 227df85..12e7f9f 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -1182,6 +1182,8 @@ UnicodeToEfiKey (
UINT16 UnicodeChar;
EFI_INPUT_KEY Key;
BOOLEAN SetDefaultResetState;
+ static UINT16 TtyEscapeStr[3];
+ static INTN TtyEscapeIndex;
Is this guaranteed to be safe?
Post by Roy Franz
TimerStatus = gBS->CheckEvent (TerminalDevice->TwoSecondTimeOut);
@@ -1223,7 +1225,8 @@ UnicodeToEfiKey (
continue;
}
- if (UnicodeChar == 'O' && TerminalDevice->TerminalType == VT100TYPE) {
+ if (UnicodeChar == 'O' && (TerminalDevice->TerminalType == VT100TYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE)) {
TerminalDevice->InputState |= INPUT_STATE_O;
TerminalDevice->ResetState = RESET_STATE_DEFAULT;
continue;
@@ -1371,6 +1374,22 @@ UnicodeToEfiKey (
break;
}
+ } else if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+ /* Also accept VT100 escape codes for F1-F4 for TTY term */
+ switch (UnicodeChar) {
+ Key.ScanCode = SCAN_F1;
+ break;
+ Key.ScanCode = SCAN_F2;
+ break;
+ Key.ScanCode = SCAN_F3;
+ break;
+ Key.ScanCode = SCAN_F4;
+ break;
+ }
}
if (Key.ScanCode != SCAN_NULL) {
@@ -1514,6 +1533,21 @@ UnicodeToEfiKey (
}
}
+ /*
+ * The VT220 escape codes that the TTY terminal accepts all have
+ * numeric codes, and there are no ambiguous prefixes shared with
+ * other terminal types.
+ */
+ if (TerminalDevice->TerminalType == TTYTERMTYPE &&
+ Key.ScanCode == SCAN_NULL &&
+ UnicodeChar >= '0' &&
+ UnicodeChar <= '9') {
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
+ TerminalDevice->InputState |= INPUT_STATE_LEFTOPENBRACKET_2;
+ continue;
+ }
+
if (Key.ScanCode != SCAN_NULL) {
Key.UnicodeChar = 0;
EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
@@ -1527,6 +1561,65 @@ UnicodeToEfiKey (
break;
+ /*
+ * Here we handle the VT220 escape codes that we accept. This
+ * state is only used by the TTY terminal type.
+ */
+ Key.ScanCode = SCAN_NULL;
+ if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+
+ if (UnicodeChar == '~' && TtyEscapeIndex <= 2) {
+ UINTN EscCode;
+ TtyEscapeStr[TtyEscapeIndex] = 0; /* Terminate string */
+ EscCode = StrDecimalToUintn(TtyEscapeStr);
+ switch (EscCode) {
+ Key.ScanCode = SCAN_DELETE;
+ break;
+ Key.ScanCode = SCAN_F1 + EscCode - 11;
+ break;
+ Key.ScanCode = SCAN_F6 + EscCode - 17;
+ break;
+ Key.ScanCode = SCAN_F11 + EscCode - 23;
+ break;
+ break;
+ }
+ } else if (TtyEscapeIndex == 1){
+ /* 2 character escape code */
+ TtyEscapeStr[TtyEscapeIndex++] = UnicodeChar;
+ continue;
+ }
+ else {
+ DEBUG ((EFI_D_ERROR, "Unexpected state in escape2\n"));
+ }
+ }
+ TerminalDevice->ResetState = RESET_STATE_DEFAULT;
+
+ if (Key.ScanCode != SCAN_NULL) {
+ Key.UnicodeChar = 0;
+ EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
+ TerminalDevice->InputState = INPUT_STATE_DEFAULT;
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ continue;
+ }
+
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ break;
+
//
// Invalid state. This should never happen.
Changing state machine code like this makes me nervous, but as far as
I can tell, non-TtyTerm users are not affected by it, so there is
little risk of regression.

Reviewed-by: Ard Biesheuvel <***@linaro.org>
Roy Franz
2015-07-07 20:07:27 UTC
Permalink
On Tue, Jul 7, 2015 at 6:18 AM, Ard Biesheuvel
Post by Ard Biesheuvel
Post by Roy Franz
Accept the VT220 escape code [3~ as backspace for TtyTerm terminals. This is
sent by many Linux terminals by default. Also accept VT220 function keys
F1-F12, and VT100 F1-F4 keys as these are commonly sent by Linux terminals.
The VT220 escape codes are longer, and variable length so a new state is added
to the state machine along with a variable to construct the multibyte escape
sequence.
There are currently no ambiguous escape sequence prefixes accepted, so the TTY
terminal accepts escape sequences for a variety of terminals. The goal is to
'just work' with as many terminals as possible, rather than properly emulating
any specific terminal. Backspace, Del, and F10 have been tested on xterm,
rxvt, tmux, and screen.
Note: The existing vt100 function key handling does not match the vt100
documentation that I found, so I added the TTY terminal handling
of VT100 F1-F4 (really PF1-PF4 on vt100) separately. The vt100
has no F5-F10 keys, so I don't know what the current vt100 code
is based on.
Contributed-under: TianoCore Contribution Agreement 1.0
Same here ^^^
(and in patch 1/4)
I'll fix this here and in the other patches. Maybe I should re-read
my contributions.txt patch so I get this right :)
Post by Ard Biesheuvel
Post by Roy Franz
---
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 1 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 95 +++++++++++++++++++-
2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 03542a4..4616ab3 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -118,6 +118,7 @@ typedef struct {
#define INPUT_STATE_LEFTOPENBRACKET 0x04
#define INPUT_STATE_O 0x08
#define INPUT_STATE_2 0x10
+#define INPUT_STATE_LEFTOPENBRACKET_2 0x20
#define RESET_STATE_DEFAULT 0x00
#define RESET_STATE_ESC_R 0x01
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 227df85..12e7f9f 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -1182,6 +1182,8 @@ UnicodeToEfiKey (
UINT16 UnicodeChar;
EFI_INPUT_KEY Key;
BOOLEAN SetDefaultResetState;
+ static UINT16 TtyEscapeStr[3];
+ static INTN TtyEscapeIndex;
Is this guaranteed to be safe?
I'm assuming you mean the length of 3? (for 2 characters + NUL termination)
I think it is.

I do the following when the INPUT_STATE_LEFTOPENBRACKET_2 is entered:
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;

When any additional escape sequence characters are processed any values
of TtyEscapeIndex other than 1 or 2 are errors. If we got an escape sequence
that was too long, we will go back to the RESET_STATE_DEFAULT state,
and TtyEscape* will be re-initialized when the INPUT_STATE_LEFTOPENBRACKET_2
state is entered.

There is only 1 place where INPUT_STATE_LEFTOPENBRACKET_2 is entered,
and both TtyEscape* variables are initialized there, and only valid
(ie Index of 1 or 2)
index values are processed once in INPUT_STATE_LEFTOPENBRACKET_2.

I can add some comments to this effect if you think it is worthwhile.
Post by Ard Biesheuvel
Post by Roy Franz
TimerStatus = gBS->CheckEvent (TerminalDevice->TwoSecondTimeOut);
@@ -1223,7 +1225,8 @@ UnicodeToEfiKey (
continue;
}
- if (UnicodeChar == 'O' && TerminalDevice->TerminalType == VT100TYPE) {
+ if (UnicodeChar == 'O' && (TerminalDevice->TerminalType == VT100TYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE)) {
TerminalDevice->InputState |= INPUT_STATE_O;
TerminalDevice->ResetState = RESET_STATE_DEFAULT;
continue;
@@ -1371,6 +1374,22 @@ UnicodeToEfiKey (
break;
}
+ } else if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+ /* Also accept VT100 escape codes for F1-F4 for TTY term */
+ switch (UnicodeChar) {
+ Key.ScanCode = SCAN_F1;
+ break;
+ Key.ScanCode = SCAN_F2;
+ break;
+ Key.ScanCode = SCAN_F3;
+ break;
+ Key.ScanCode = SCAN_F4;
+ break;
+ }
}
if (Key.ScanCode != SCAN_NULL) {
@@ -1514,6 +1533,21 @@ UnicodeToEfiKey (
}
}
+ /*
+ * The VT220 escape codes that the TTY terminal accepts all have
+ * numeric codes, and there are no ambiguous prefixes shared with
+ * other terminal types.
+ */
+ if (TerminalDevice->TerminalType == TTYTERMTYPE &&
+ Key.ScanCode == SCAN_NULL &&
+ UnicodeChar >= '0' &&
+ UnicodeChar <= '9') {
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
+ TerminalDevice->InputState |= INPUT_STATE_LEFTOPENBRACKET_2;
+ continue;
+ }
+
if (Key.ScanCode != SCAN_NULL) {
Key.UnicodeChar = 0;
EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
@@ -1527,6 +1561,65 @@ UnicodeToEfiKey (
break;
+ /*
+ * Here we handle the VT220 escape codes that we accept. This
+ * state is only used by the TTY terminal type.
+ */
+ Key.ScanCode = SCAN_NULL;
+ if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+
+ if (UnicodeChar == '~' && TtyEscapeIndex <= 2) {
+ UINTN EscCode;
+ TtyEscapeStr[TtyEscapeIndex] = 0; /* Terminate string */
+ EscCode = StrDecimalToUintn(TtyEscapeStr);
+ switch (EscCode) {
+ Key.ScanCode = SCAN_DELETE;
+ break;
+ Key.ScanCode = SCAN_F1 + EscCode - 11;
+ break;
+ Key.ScanCode = SCAN_F6 + EscCode - 17;
+ break;
+ Key.ScanCode = SCAN_F11 + EscCode - 23;
+ break;
+ break;
+ }
+ } else if (TtyEscapeIndex == 1){
+ /* 2 character escape code */
+ TtyEscapeStr[TtyEscapeIndex++] = UnicodeChar;
+ continue;
+ }
+ else {
+ DEBUG ((EFI_D_ERROR, "Unexpected state in escape2\n"));
+ }
+ }
+ TerminalDevice->ResetState = RESET_STATE_DEFAULT;
+
+ if (Key.ScanCode != SCAN_NULL) {
+ Key.UnicodeChar = 0;
+ EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
+ TerminalDevice->InputState = INPUT_STATE_DEFAULT;
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ continue;
+ }
+
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ break;
+
//
// Invalid state. This should never happen.
Changing state machine code like this makes me nervous, but as far as
I can tell, non-TtyTerm users are not affected by it, so there is
little risk of regression.
Ard Biesheuvel
2015-07-07 20:25:10 UTC
Permalink
Post by Roy Franz
On Tue, Jul 7, 2015 at 6:18 AM, Ard Biesheuvel
Post by Ard Biesheuvel
Post by Roy Franz
Accept the VT220 escape code [3~ as backspace for TtyTerm terminals. This is
sent by many Linux terminals by default. Also accept VT220 function keys
F1-F12, and VT100 F1-F4 keys as these are commonly sent by Linux terminals.
The VT220 escape codes are longer, and variable length so a new state is added
to the state machine along with a variable to construct the multibyte escape
sequence.
There are currently no ambiguous escape sequence prefixes accepted, so the TTY
terminal accepts escape sequences for a variety of terminals. The goal is to
'just work' with as many terminals as possible, rather than properly emulating
any specific terminal. Backspace, Del, and F10 have been tested on xterm,
rxvt, tmux, and screen.
Note: The existing vt100 function key handling does not match the vt100
documentation that I found, so I added the TTY terminal handling
of VT100 F1-F4 (really PF1-PF4 on vt100) separately. The vt100
has no F5-F10 keys, so I don't know what the current vt100 code
is based on.
Contributed-under: TianoCore Contribution Agreement 1.0
Same here ^^^
(and in patch 1/4)
I'll fix this here and in the other patches. Maybe I should re-read
my contributions.txt patch so I get this right :)
:-)
Post by Roy Franz
Post by Ard Biesheuvel
Post by Roy Franz
---
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 1 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 95 +++++++++++++++++++-
2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 03542a4..4616ab3 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -118,6 +118,7 @@ typedef struct {
#define INPUT_STATE_LEFTOPENBRACKET 0x04
#define INPUT_STATE_O 0x08
#define INPUT_STATE_2 0x10
+#define INPUT_STATE_LEFTOPENBRACKET_2 0x20
#define RESET_STATE_DEFAULT 0x00
#define RESET_STATE_ESC_R 0x01
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 227df85..12e7f9f 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -1182,6 +1182,8 @@ UnicodeToEfiKey (
UINT16 UnicodeChar;
EFI_INPUT_KEY Key;
BOOLEAN SetDefaultResetState;
+ static UINT16 TtyEscapeStr[3];
+ static INTN TtyEscapeIndex;
Is this guaranteed to be safe?
I'm assuming you mean the length of 3? (for 2 characters + NUL termination)
I think it is.
No, I mean the static. I know UEFI is single threaded, but it has an
event model that could potentially result in functions being called in
a reentrant fashion. I think this can only happen in functions that
signal events themselves, but I am not intimate enough with this stuff
to claim that this is 100% safe.
Post by Roy Franz
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
When any additional escape sequence characters are processed any values
of TtyEscapeIndex other than 1 or 2 are errors. If we got an escape sequence
that was too long, we will go back to the RESET_STATE_DEFAULT state,
and TtyEscape* will be re-initialized when the INPUT_STATE_LEFTOPENBRACKET_2
state is entered.
There is only 1 place where INPUT_STATE_LEFTOPENBRACKET_2 is entered,
and both TtyEscape* variables are initialized there, and only valid
(ie Index of 1 or 2)
index values are processed once in INPUT_STATE_LEFTOPENBRACKET_2.
I can add some comments to this effect if you think it is worthwhile.
Post by Ard Biesheuvel
Post by Roy Franz
TimerStatus = gBS->CheckEvent (TerminalDevice->TwoSecondTimeOut);
@@ -1223,7 +1225,8 @@ UnicodeToEfiKey (
continue;
}
- if (UnicodeChar == 'O' && TerminalDevice->TerminalType == VT100TYPE) {
+ if (UnicodeChar == 'O' && (TerminalDevice->TerminalType == VT100TYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE)) {
TerminalDevice->InputState |= INPUT_STATE_O;
TerminalDevice->ResetState = RESET_STATE_DEFAULT;
continue;
@@ -1371,6 +1374,22 @@ UnicodeToEfiKey (
break;
}
+ } else if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+ /* Also accept VT100 escape codes for F1-F4 for TTY term */
+ switch (UnicodeChar) {
+ Key.ScanCode = SCAN_F1;
+ break;
+ Key.ScanCode = SCAN_F2;
+ break;
+ Key.ScanCode = SCAN_F3;
+ break;
+ Key.ScanCode = SCAN_F4;
+ break;
+ }
}
if (Key.ScanCode != SCAN_NULL) {
@@ -1514,6 +1533,21 @@ UnicodeToEfiKey (
}
}
+ /*
+ * The VT220 escape codes that the TTY terminal accepts all have
+ * numeric codes, and there are no ambiguous prefixes shared with
+ * other terminal types.
+ */
+ if (TerminalDevice->TerminalType == TTYTERMTYPE &&
+ Key.ScanCode == SCAN_NULL &&
+ UnicodeChar >= '0' &&
+ UnicodeChar <= '9') {
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
+ TerminalDevice->InputState |= INPUT_STATE_LEFTOPENBRACKET_2;
+ continue;
+ }
+
if (Key.ScanCode != SCAN_NULL) {
Key.UnicodeChar = 0;
EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
@@ -1527,6 +1561,65 @@ UnicodeToEfiKey (
break;
+ /*
+ * Here we handle the VT220 escape codes that we accept. This
+ * state is only used by the TTY terminal type.
+ */
+ Key.ScanCode = SCAN_NULL;
+ if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+
+ if (UnicodeChar == '~' && TtyEscapeIndex <= 2) {
+ UINTN EscCode;
+ TtyEscapeStr[TtyEscapeIndex] = 0; /* Terminate string */
+ EscCode = StrDecimalToUintn(TtyEscapeStr);
+ switch (EscCode) {
+ Key.ScanCode = SCAN_DELETE;
+ break;
+ Key.ScanCode = SCAN_F1 + EscCode - 11;
+ break;
+ Key.ScanCode = SCAN_F6 + EscCode - 17;
+ break;
+ Key.ScanCode = SCAN_F11 + EscCode - 23;
+ break;
+ break;
+ }
+ } else if (TtyEscapeIndex == 1){
+ /* 2 character escape code */
+ TtyEscapeStr[TtyEscapeIndex++] = UnicodeChar;
+ continue;
+ }
+ else {
+ DEBUG ((EFI_D_ERROR, "Unexpected state in escape2\n"));
+ }
+ }
+ TerminalDevice->ResetState = RESET_STATE_DEFAULT;
+
+ if (Key.ScanCode != SCAN_NULL) {
+ Key.UnicodeChar = 0;
+ EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
+ TerminalDevice->InputState = INPUT_STATE_DEFAULT;
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ continue;
+ }
+
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ break;
+
//
// Invalid state. This should never happen.
Changing state machine code like this makes me nervous, but as far as
I can tell, non-TtyTerm users are not affected by it, so there is
little risk of regression.
Roy Franz
2015-07-07 20:31:10 UTC
Permalink
On Tue, Jul 7, 2015 at 1:25 PM, Ard Biesheuvel
Post by Ard Biesheuvel
Post by Roy Franz
On Tue, Jul 7, 2015 at 6:18 AM, Ard Biesheuvel
Post by Ard Biesheuvel
Post by Roy Franz
Accept the VT220 escape code [3~ as backspace for TtyTerm terminals. This is
sent by many Linux terminals by default. Also accept VT220 function keys
F1-F12, and VT100 F1-F4 keys as these are commonly sent by Linux terminals.
The VT220 escape codes are longer, and variable length so a new state is added
to the state machine along with a variable to construct the multibyte escape
sequence.
There are currently no ambiguous escape sequence prefixes accepted, so the TTY
terminal accepts escape sequences for a variety of terminals. The goal is to
'just work' with as many terminals as possible, rather than properly emulating
any specific terminal. Backspace, Del, and F10 have been tested on xterm,
rxvt, tmux, and screen.
Note: The existing vt100 function key handling does not match the vt100
documentation that I found, so I added the TTY terminal handling
of VT100 F1-F4 (really PF1-PF4 on vt100) separately. The vt100
has no F5-F10 keys, so I don't know what the current vt100 code
is based on.
Contributed-under: TianoCore Contribution Agreement 1.0
Same here ^^^
(and in patch 1/4)
I'll fix this here and in the other patches. Maybe I should re-read
my contributions.txt patch so I get this right :)
:-)
Post by Roy Franz
Post by Ard Biesheuvel
Post by Roy Franz
---
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 1 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 95 +++++++++++++++++++-
2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 03542a4..4616ab3 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -118,6 +118,7 @@ typedef struct {
#define INPUT_STATE_LEFTOPENBRACKET 0x04
#define INPUT_STATE_O 0x08
#define INPUT_STATE_2 0x10
+#define INPUT_STATE_LEFTOPENBRACKET_2 0x20
#define RESET_STATE_DEFAULT 0x00
#define RESET_STATE_ESC_R 0x01
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 227df85..12e7f9f 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -1182,6 +1182,8 @@ UnicodeToEfiKey (
UINT16 UnicodeChar;
EFI_INPUT_KEY Key;
BOOLEAN SetDefaultResetState;
+ static UINT16 TtyEscapeStr[3];
+ static INTN TtyEscapeIndex;
Is this guaranteed to be safe?
I'm assuming you mean the length of 3? (for 2 characters + NUL termination)
I think it is.
No, I mean the static. I know UEFI is single threaded, but it has an
event model that could potentially result in functions being called in
a reentrant fashion. I think this can only happen in functions that
signal events themselves, but I am not intimate enough with this stuff
to claim that this is 100% safe.
Hmm, that is a good point. I think these should really go into the
TerminalDevice
structure, since these really are per-terminal state. I'll move them
there, as that is
where the rest of the terminal state is tracked.
Post by Ard Biesheuvel
Post by Roy Franz
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
When any additional escape sequence characters are processed any values
of TtyEscapeIndex other than 1 or 2 are errors. If we got an escape sequence
that was too long, we will go back to the RESET_STATE_DEFAULT state,
and TtyEscape* will be re-initialized when the INPUT_STATE_LEFTOPENBRACKET_2
state is entered.
There is only 1 place where INPUT_STATE_LEFTOPENBRACKET_2 is entered,
and both TtyEscape* variables are initialized there, and only valid
(ie Index of 1 or 2)
index values are processed once in INPUT_STATE_LEFTOPENBRACKET_2.
I can add some comments to this effect if you think it is worthwhile.
Post by Ard Biesheuvel
Post by Roy Franz
TimerStatus = gBS->CheckEvent (TerminalDevice->TwoSecondTimeOut);
@@ -1223,7 +1225,8 @@ UnicodeToEfiKey (
continue;
}
- if (UnicodeChar == 'O' && TerminalDevice->TerminalType == VT100TYPE) {
+ if (UnicodeChar == 'O' && (TerminalDevice->TerminalType == VT100TYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE)) {
TerminalDevice->InputState |= INPUT_STATE_O;
TerminalDevice->ResetState = RESET_STATE_DEFAULT;
continue;
@@ -1371,6 +1374,22 @@ UnicodeToEfiKey (
break;
}
+ } else if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+ /* Also accept VT100 escape codes for F1-F4 for TTY term */
+ switch (UnicodeChar) {
+ Key.ScanCode = SCAN_F1;
+ break;
+ Key.ScanCode = SCAN_F2;
+ break;
+ Key.ScanCode = SCAN_F3;
+ break;
+ Key.ScanCode = SCAN_F4;
+ break;
+ }
}
if (Key.ScanCode != SCAN_NULL) {
@@ -1514,6 +1533,21 @@ UnicodeToEfiKey (
}
}
+ /*
+ * The VT220 escape codes that the TTY terminal accepts all have
+ * numeric codes, and there are no ambiguous prefixes shared with
+ * other terminal types.
+ */
+ if (TerminalDevice->TerminalType == TTYTERMTYPE &&
+ Key.ScanCode == SCAN_NULL &&
+ UnicodeChar >= '0' &&
+ UnicodeChar <= '9') {
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
+ TerminalDevice->InputState |= INPUT_STATE_LEFTOPENBRACKET_2;
+ continue;
+ }
+
if (Key.ScanCode != SCAN_NULL) {
Key.UnicodeChar = 0;
EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
@@ -1527,6 +1561,65 @@ UnicodeToEfiKey (
break;
+ /*
+ * Here we handle the VT220 escape codes that we accept. This
+ * state is only used by the TTY terminal type.
+ */
+ Key.ScanCode = SCAN_NULL;
+ if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+
+ if (UnicodeChar == '~' && TtyEscapeIndex <= 2) {
+ UINTN EscCode;
+ TtyEscapeStr[TtyEscapeIndex] = 0; /* Terminate string */
+ EscCode = StrDecimalToUintn(TtyEscapeStr);
+ switch (EscCode) {
+ Key.ScanCode = SCAN_DELETE;
+ break;
+ Key.ScanCode = SCAN_F1 + EscCode - 11;
+ break;
+ Key.ScanCode = SCAN_F6 + EscCode - 17;
+ break;
+ Key.ScanCode = SCAN_F11 + EscCode - 23;
+ break;
+ break;
+ }
+ } else if (TtyEscapeIndex == 1){
+ /* 2 character escape code */
+ TtyEscapeStr[TtyEscapeIndex++] = UnicodeChar;
+ continue;
+ }
+ else {
+ DEBUG ((EFI_D_ERROR, "Unexpected state in escape2\n"));
+ }
+ }
+ TerminalDevice->ResetState = RESET_STATE_DEFAULT;
+
+ if (Key.ScanCode != SCAN_NULL) {
+ Key.UnicodeChar = 0;
+ EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
+ TerminalDevice->InputState = INPUT_STATE_DEFAULT;
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ continue;
+ }
+
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ break;
+
//
// Invalid state. This should never happen.
Changing state machine code like this makes me nervous, but as far as
I can tell, non-TtyTerm users are not affected by it, so there is
little risk of regression.
Andrew Fish
2015-07-07 20:41:24 UTC
Permalink
Post by Roy Franz
On Tue, Jul 7, 2015 at 1:25 PM, Ard Biesheuvel
Post by Ard Biesheuvel
Post by Roy Franz
On Tue, Jul 7, 2015 at 6:18 AM, Ard Biesheuvel
Post by Ard Biesheuvel
Post by Roy Franz
Accept the VT220 escape code [3~ as backspace for TtyTerm terminals. This is
sent by many Linux terminals by default. Also accept VT220 function keys
F1-F12, and VT100 F1-F4 keys as these are commonly sent by Linux terminals.
The VT220 escape codes are longer, and variable length so a new state is added
to the state machine along with a variable to construct the multibyte escape
sequence.
There are currently no ambiguous escape sequence prefixes accepted, so the TTY
terminal accepts escape sequences for a variety of terminals. The goal is to
'just work' with as many terminals as possible, rather than properly emulating
any specific terminal. Backspace, Del, and F10 have been tested on xterm,
rxvt, tmux, and screen.
Note: The existing vt100 function key handling does not match the vt100
documentation that I found, so I added the TTY terminal handling
of VT100 F1-F4 (really PF1-PF4 on vt100) separately. The vt100
has no F5-F10 keys, so I don't know what the current vt100 code
is based on.
Contributed-under: TianoCore Contribution Agreement 1.0
Same here ^^^
(and in patch 1/4)
I'll fix this here and in the other patches. Maybe I should re-read
my contributions.txt patch so I get this right :)
:-)
Post by Roy Franz
Post by Ard Biesheuvel
Post by Roy Franz
---
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 1 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 95 +++++++++++++++++++-
2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 03542a4..4616ab3 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -118,6 +118,7 @@ typedef struct {
#define INPUT_STATE_LEFTOPENBRACKET 0x04
#define INPUT_STATE_O 0x08
#define INPUT_STATE_2 0x10
+#define INPUT_STATE_LEFTOPENBRACKET_2 0x20
#define RESET_STATE_DEFAULT 0x00
#define RESET_STATE_ESC_R 0x01
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 227df85..12e7f9f 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -1182,6 +1182,8 @@ UnicodeToEfiKey (
UINT16 UnicodeChar;
EFI_INPUT_KEY Key;
BOOLEAN SetDefaultResetState;
+ static UINT16 TtyEscapeStr[3];
+ static INTN TtyEscapeIndex;
Is this guaranteed to be safe?
I'm assuming you mean the length of 3? (for 2 characters + NUL termination)
I think it is.
No, I mean the static. I know UEFI is single threaded, but it has an
event model that could potentially result in functions being called in
a reentrant fashion. I think this can only happen in functions that
signal events themselves, but I am not intimate enough with this stuff
to claim that this is 100% safe.
Hmm, that is a good point. I think these should really go into the
TerminalDevice
In general globals (or static variables in a function) are bad as there can be multiple instances of a protocol. The terminal driver could be connected on an arbitrary number of serial ports.
Post by Roy Franz
structure, since these really are per-terminal state. I'll move them
there, as that is
where the rest of the terminal state is tracked.
This is the data structure that tracks the Protocol’s instance data. The generic form in EFI is to use the Containment Record (CR()) macro to convert the protocol instance back into the private data for the driver.

https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h <https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h>
typedef struct {
UINTN Signature;
EFI_HANDLE Handle;
UINT8 TerminalType;
EFI_SERIAL_IO_PROTOCOL *SerialIo;
EFI_DEVICE_PATH_PROTOCOL *DevicePath;
EFI_SIMPLE_TEXT_INPUT_PROTOCOL SimpleInput;
EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL SimpleTextOutput;
EFI_SIMPLE_TEXT_OUTPUT_MODE SimpleTextOutputMode;
TERMINAL_CONSOLE_MODE_DATA *TerminalConsoleModeData;
UINTN SerialInTimeOut;
RAW_DATA_FIFO *RawFiFo;
UNICODE_FIFO *UnicodeFiFo;
EFI_KEY_FIFO *EfiKeyFiFo;
EFI_UNICODE_STRING_TABLE *ControllerNameTable;
EFI_EVENT TimerEvent;
EFI_EVENT TwoSecondTimeOut;
UINT32 InputState;
UINT32 ResetState;

//
// Esc could not be output to the screen by user,
// but the terminal driver need to output it to
// the terminal emulation software to send control sequence.
// This boolean is used by the terminal driver only
// to indicate whether the Esc could be sent or not.
//
BOOLEAN OutputEscChar;
EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL SimpleInputEx;
LIST_ENTRY NotifyList;
} TERMINAL_DEV;


#define TERMINAL_CON_IN_DEV_FROM_THIS(a) CR (a, TERMINAL_DEV, SimpleInput, TERMINAL_DEV_SIGNATURE)
#define TERMINAL_CON_OUT_DEV_FROM_THIS(a) CR (a, TERMINAL_DEV, SimpleTextOutput, TERMINAL_DEV_SIGNATURE)
#define TERMINAL_CON_IN_EX_DEV_FROM_THIS(a) CR (a, TERMINAL_DEV, SimpleInputEx, TERMINAL_DEV_SIGNATURE)
Thanks,

Andrew Fish
Post by Roy Franz
Post by Ard Biesheuvel
Post by Roy Franz
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
When any additional escape sequence characters are processed any values
of TtyEscapeIndex other than 1 or 2 are errors. If we got an escape sequence
that was too long, we will go back to the RESET_STATE_DEFAULT state,
and TtyEscape* will be re-initialized when the INPUT_STATE_LEFTOPENBRACKET_2
state is entered.
There is only 1 place where INPUT_STATE_LEFTOPENBRACKET_2 is entered,
and both TtyEscape* variables are initialized there, and only valid
(ie Index of 1 or 2)
index values are processed once in INPUT_STATE_LEFTOPENBRACKET_2.
I can add some comments to this effect if you think it is worthwhile.
Post by Ard Biesheuvel
Post by Roy Franz
TimerStatus = gBS->CheckEvent (TerminalDevice->TwoSecondTimeOut);
@@ -1223,7 +1225,8 @@ UnicodeToEfiKey (
continue;
}
- if (UnicodeChar == 'O' && TerminalDevice->TerminalType == VT100TYPE) {
+ if (UnicodeChar == 'O' && (TerminalDevice->TerminalType == VT100TYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE)) {
TerminalDevice->InputState |= INPUT_STATE_O;
TerminalDevice->ResetState = RESET_STATE_DEFAULT;
continue;
@@ -1371,6 +1374,22 @@ UnicodeToEfiKey (
break;
}
+ } else if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+ /* Also accept VT100 escape codes for F1-F4 for TTY term */
+ switch (UnicodeChar) {
+ Key.ScanCode = SCAN_F1;
+ break;
+ Key.ScanCode = SCAN_F2;
+ break;
+ Key.ScanCode = SCAN_F3;
+ break;
+ Key.ScanCode = SCAN_F4;
+ break;
+ }
}
if (Key.ScanCode != SCAN_NULL) {
@@ -1514,6 +1533,21 @@ UnicodeToEfiKey (
}
}
+ /*
+ * The VT220 escape codes that the TTY terminal accepts all have
+ * numeric codes, and there are no ambiguous prefixes shared with
+ * other terminal types.
+ */
+ if (TerminalDevice->TerminalType == TTYTERMTYPE &&
+ Key.ScanCode == SCAN_NULL &&
+ UnicodeChar >= '0' &&
+ UnicodeChar <= '9') {
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
+ TerminalDevice->InputState |= INPUT_STATE_LEFTOPENBRACKET_2;
+ continue;
+ }
+
if (Key.ScanCode != SCAN_NULL) {
Key.UnicodeChar = 0;
EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
@@ -1527,6 +1561,65 @@ UnicodeToEfiKey (
break;
+ /*
+ * Here we handle the VT220 escape codes that we accept. This
+ * state is only used by the TTY terminal type.
+ */
+ Key.ScanCode = SCAN_NULL;
+ if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+
+ if (UnicodeChar == '~' && TtyEscapeIndex <= 2) {
+ UINTN EscCode;
+ TtyEscapeStr[TtyEscapeIndex] = 0; /* Terminate string */
+ EscCode = StrDecimalToUintn(TtyEscapeStr);
+ switch (EscCode) {
+ Key.ScanCode = SCAN_DELETE;
+ break;
+ Key.ScanCode = SCAN_F1 + EscCode - 11;
+ break;
+ Key.ScanCode = SCAN_F6 + EscCode - 17;
+ break;
+ Key.ScanCode = SCAN_F11 + EscCode - 23;
+ break;
+ break;
+ }
+ } else if (TtyEscapeIndex == 1){
+ /* 2 character escape code */
+ TtyEscapeStr[TtyEscapeIndex++] = UnicodeChar;
+ continue;
+ }
+ else {
+ DEBUG ((EFI_D_ERROR, "Unexpected state in escape2\n"));
+ }
+ }
+ TerminalDevice->ResetState = RESET_STATE_DEFAULT;
+
+ if (Key.ScanCode != SCAN_NULL) {
+ Key.UnicodeChar = 0;
+ EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
+ TerminalDevice->InputState = INPUT_STATE_DEFAULT;
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ continue;
+ }
+
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ break;
+
//
// Invalid state. This should never happen.
Changing state machine code like this makes me nervous, but as far as
I can tell, non-TtyTerm users are not affected by it, so there is
little risk of regression.
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Roy Franz
2015-07-07 21:12:41 UTC
Permalink
Post by Roy Franz
On Tue, Jul 7, 2015 at 1:25 PM, Ard Biesheuvel
On Tue, Jul 7, 2015 at 6:18 AM, Ard Biesheuvel
Accept the VT220 escape code [3~ as backspace for TtyTerm terminals. This is
sent by many Linux terminals by default. Also accept VT220 function keys
F1-F12, and VT100 F1-F4 keys as these are commonly sent by Linux terminals.
The VT220 escape codes are longer, and variable length so a new state is added
to the state machine along with a variable to construct the multibyte escape
sequence.
There are currently no ambiguous escape sequence prefixes accepted, so the TTY
terminal accepts escape sequences for a variety of terminals. The goal is to
'just work' with as many terminals as possible, rather than properly emulating
any specific terminal. Backspace, Del, and F10 have been tested on xterm,
rxvt, tmux, and screen.
Note: The existing vt100 function key handling does not match the vt100
documentation that I found, so I added the TTY terminal handling
of VT100 F1-F4 (really PF1-PF4 on vt100) separately. The vt100
has no F5-F10 keys, so I don't know what the current vt100 code
is based on.
Contributed-under: TianoCore Contribution Agreement 1.0
Same here ^^^
(and in patch 1/4)
I'll fix this here and in the other patches. Maybe I should re-read
my contributions.txt patch so I get this right :)
:-)
---
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 1 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 95 +++++++++++++++++++-
2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 03542a4..4616ab3 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -118,6 +118,7 @@ typedef struct {
#define INPUT_STATE_LEFTOPENBRACKET 0x04
#define INPUT_STATE_O 0x08
#define INPUT_STATE_2 0x10
+#define INPUT_STATE_LEFTOPENBRACKET_2 0x20
#define RESET_STATE_DEFAULT 0x00
#define RESET_STATE_ESC_R 0x01
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 227df85..12e7f9f 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -1182,6 +1182,8 @@ UnicodeToEfiKey (
UINT16 UnicodeChar;
EFI_INPUT_KEY Key;
BOOLEAN SetDefaultResetState;
+ static UINT16 TtyEscapeStr[3];
+ static INTN TtyEscapeIndex;
Is this guaranteed to be safe?
I'm assuming you mean the length of 3? (for 2 characters + NUL termination)
I think it is.
No, I mean the static. I know UEFI is single threaded, but it has an
event model that could potentially result in functions being called in
a reentrant fashion. I think this can only happen in functions that
signal events themselves, but I am not intimate enough with this stuff
to claim that this is 100% safe.
Hmm, that is a good point. I think these should really go into the
TerminalDevice
In general globals (or static variables in a function) are bad as there can
be multiple instances of a protocol. The terminal driver could be connected
on an arbitrary number of serial ports.
structure, since these really are per-terminal state. I'll move them
there, as that is
where the rest of the terminal state is tracked.
This is the data structure that tracks the Protocol’s instance data. The
generic form in EFI is to use the Containment Record (CR()) macro to convert
the protocol instance back into the private data for the driver.
https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
typedef struct {
UINTN Signature;
EFI_HANDLE Handle;
UINT8 TerminalType;
EFI_SERIAL_IO_PROTOCOL *SerialIo;
EFI_DEVICE_PATH_PROTOCOL *DevicePath;
EFI_SIMPLE_TEXT_INPUT_PROTOCOL SimpleInput;
EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL SimpleTextOutput;
EFI_SIMPLE_TEXT_OUTPUT_MODE SimpleTextOutputMode;
TERMINAL_CONSOLE_MODE_DATA *TerminalConsoleModeData;
UINTN SerialInTimeOut;
RAW_DATA_FIFO *RawFiFo;
UNICODE_FIFO *UnicodeFiFo;
EFI_KEY_FIFO *EfiKeyFiFo;
EFI_UNICODE_STRING_TABLE *ControllerNameTable;
EFI_EVENT TimerEvent;
EFI_EVENT TwoSecondTimeOut;
UINT32 InputState;
UINT32 ResetState;
//
// Esc could not be output to the screen by user,
// but the terminal driver need to output it to
// the terminal emulation software to send control sequence.
// This boolean is used by the terminal driver only
// to indicate whether the Esc could be sent or not.
//
BOOLEAN OutputEscChar;
EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL SimpleInputEx;
LIST_ENTRY NotifyList;
} TERMINAL_DEV;
…
Agreed, the static was clearly wrong.
Post by Roy Franz
#define TERMINAL_CON_IN_DEV_FROM_THIS(a) CR (a, TERMINAL_DEV, SimpleInput,
TERMINAL_DEV_SIGNATURE)
#define TERMINAL_CON_OUT_DEV_FROM_THIS(a) CR (a, TERMINAL_DEV,
SimpleTextOutput, TERMINAL_DEV_SIGNATURE)
#define TERMINAL_CON_IN_EX_DEV_FROM_THIS(a) CR (a, TERMINAL_DEV,
SimpleInputEx, TERMINAL_DEV_SIGNATURE)
In UnicodeToEfiKey() we are passed a TerminalDevice structure already, so the
TERMINAL_CON_IN_DEV_FROM_THIS() has already been done for us.

I'll post an updated series that fixes this (and a few other more
minor changes) shortly.

Roy
Post by Roy Franz
Thanks,
Andrew Fish
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
When any additional escape sequence characters are processed any values
of TtyEscapeIndex other than 1 or 2 are errors. If we got an escape sequence
that was too long, we will go back to the RESET_STATE_DEFAULT state,
and TtyEscape* will be re-initialized when the INPUT_STATE_LEFTOPENBRACKET_2
state is entered.
There is only 1 place where INPUT_STATE_LEFTOPENBRACKET_2 is entered,
and both TtyEscape* variables are initialized there, and only valid
(ie Index of 1 or 2)
index values are processed once in INPUT_STATE_LEFTOPENBRACKET_2.
I can add some comments to this effect if you think it is worthwhile.
TimerStatus = gBS->CheckEvent (TerminalDevice->TwoSecondTimeOut);
@@ -1223,7 +1225,8 @@ UnicodeToEfiKey (
continue;
}
- if (UnicodeChar == 'O' && TerminalDevice->TerminalType == VT100TYPE) {
+ if (UnicodeChar == 'O' && (TerminalDevice->TerminalType == VT100TYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE)) {
TerminalDevice->InputState |= INPUT_STATE_O;
TerminalDevice->ResetState = RESET_STATE_DEFAULT;
continue;
@@ -1371,6 +1374,22 @@ UnicodeToEfiKey (
break;
}
+ } else if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+ /* Also accept VT100 escape codes for F1-F4 for TTY term */
+ switch (UnicodeChar) {
+ Key.ScanCode = SCAN_F1;
+ break;
+ Key.ScanCode = SCAN_F2;
+ break;
+ Key.ScanCode = SCAN_F3;
+ break;
+ Key.ScanCode = SCAN_F4;
+ break;
+ }
}
if (Key.ScanCode != SCAN_NULL) {
@@ -1514,6 +1533,21 @@ UnicodeToEfiKey (
}
}
+ /*
+ * The VT220 escape codes that the TTY terminal accepts all have
+ * numeric codes, and there are no ambiguous prefixes shared with
+ * other terminal types.
+ */
+ if (TerminalDevice->TerminalType == TTYTERMTYPE &&
+ Key.ScanCode == SCAN_NULL &&
+ UnicodeChar >= '0' &&
+ UnicodeChar <= '9') {
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
+ TerminalDevice->InputState |= INPUT_STATE_LEFTOPENBRACKET_2;
+ continue;
+ }
+
if (Key.ScanCode != SCAN_NULL) {
Key.UnicodeChar = 0;
EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
@@ -1527,6 +1561,65 @@ UnicodeToEfiKey (
break;
+ case INPUT_STATE_ESC | INPUT_STATE_LEFTOPENBRACKET |
+ /*
+ * Here we handle the VT220 escape codes that we accept. This
+ * state is only used by the TTY terminal type.
+ */
+ Key.ScanCode = SCAN_NULL;
+ if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+
+ if (UnicodeChar == '~' && TtyEscapeIndex <= 2) {
+ UINTN EscCode;
+ TtyEscapeStr[TtyEscapeIndex] = 0; /* Terminate string */
+ EscCode = StrDecimalToUintn(TtyEscapeStr);
+ switch (EscCode) {
+ Key.ScanCode = SCAN_DELETE;
+ break;
+ Key.ScanCode = SCAN_F1 + EscCode - 11;
+ break;
+ Key.ScanCode = SCAN_F6 + EscCode - 17;
+ break;
+ Key.ScanCode = SCAN_F11 + EscCode - 23;
+ break;
+ break;
+ }
+ } else if (TtyEscapeIndex == 1){
+ /* 2 character escape code */
+ TtyEscapeStr[TtyEscapeIndex++] = UnicodeChar;
+ continue;
+ }
+ else {
+ DEBUG ((EFI_D_ERROR, "Unexpected state in escape2\n"));
+ }
+ }
+ TerminalDevice->ResetState = RESET_STATE_DEFAULT;
+
+ if (Key.ScanCode != SCAN_NULL) {
+ Key.UnicodeChar = 0;
+ EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
+ TerminalDevice->InputState = INPUT_STATE_DEFAULT;
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ continue;
+ }
+
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ break;
+
//
// Invalid state. This should never happen.
Changing state machine code like this makes me nervous, but as far as
I can tell, non-TtyTerm users are not affected by it, so there is
little risk of regression.
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Laszlo Ersek
2015-07-07 20:46:29 UTC
Permalink
Post by Ard Biesheuvel
Post by Roy Franz
On Tue, Jul 7, 2015 at 6:18 AM, Ard Biesheuvel
Post by Ard Biesheuvel
Post by Roy Franz
Accept the VT220 escape code [3~ as backspace for TtyTerm terminals. This is
sent by many Linux terminals by default. Also accept VT220 function keys
F1-F12, and VT100 F1-F4 keys as these are commonly sent by Linux terminals.
The VT220 escape codes are longer, and variable length so a new state is added
to the state machine along with a variable to construct the multibyte escape
sequence.
There are currently no ambiguous escape sequence prefixes accepted, so the TTY
terminal accepts escape sequences for a variety of terminals. The goal is to
'just work' with as many terminals as possible, rather than properly emulating
any specific terminal. Backspace, Del, and F10 have been tested on xterm,
rxvt, tmux, and screen.
Note: The existing vt100 function key handling does not match the vt100
documentation that I found, so I added the TTY terminal handling
of VT100 F1-F4 (really PF1-PF4 on vt100) separately. The vt100
has no F5-F10 keys, so I don't know what the current vt100 code
is based on.
Contributed-under: TianoCore Contribution Agreement 1.0
Same here ^^^
(and in patch 1/4)
I'll fix this here and in the other patches. Maybe I should re-read
my contributions.txt patch so I get this right :)
:-)
Post by Roy Franz
Post by Ard Biesheuvel
Post by Roy Franz
---
MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 1 +
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c | 95 +++++++++++++++++++-
2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
index 03542a4..4616ab3 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
@@ -118,6 +118,7 @@ typedef struct {
#define INPUT_STATE_LEFTOPENBRACKET 0x04
#define INPUT_STATE_O 0x08
#define INPUT_STATE_2 0x10
+#define INPUT_STATE_LEFTOPENBRACKET_2 0x20
#define RESET_STATE_DEFAULT 0x00
#define RESET_STATE_ESC_R 0x01
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
index 227df85..12e7f9f 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
@@ -1182,6 +1182,8 @@ UnicodeToEfiKey (
UINT16 UnicodeChar;
EFI_INPUT_KEY Key;
BOOLEAN SetDefaultResetState;
+ static UINT16 TtyEscapeStr[3];
+ static INTN TtyEscapeIndex;
Is this guaranteed to be safe?
I'm assuming you mean the length of 3? (for 2 characters + NUL termination)
I think it is.
No, I mean the static. I know UEFI is single threaded, but it has an
event model that could potentially result in functions being called in
a reentrant fashion. I think this can only happen in functions that
signal events themselves, but I am not intimate enough with this stuff
to claim that this is 100% safe.
Sorry, I have not been following the discussion of these three "other"
patches. But, we can see that the call chain for this function is:

TerminalConInTimerHandler()
TranslateRawDataToEfiKey()
UnicodeToEfiKey()

and TerminalConInTimerHandler() is set up as a periodic callback in
TerminalDriverBindingStart(). (That is, the Start() method of the UEFI
driver binding protocol.)

Furthermore, the context for TerminalConInTimerHandler() is
"TerminalDevice" (of type TERMINAL_DEV), which is the wrapper structure
("container") for the bunch of protocol interfaces that the driver
implements for a given terminal *instance*.

This means that all long-lived state, handled in UnicodeToEfiKey(), must
be terminal instance specific, and live inside the TERMINAL_DEV structure.

Thanks
Laszlo
Post by Ard Biesheuvel
Post by Roy Franz
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
When any additional escape sequence characters are processed any values
of TtyEscapeIndex other than 1 or 2 are errors. If we got an escape sequence
that was too long, we will go back to the RESET_STATE_DEFAULT state,
and TtyEscape* will be re-initialized when the INPUT_STATE_LEFTOPENBRACKET_2
state is entered.
There is only 1 place where INPUT_STATE_LEFTOPENBRACKET_2 is entered,
and both TtyEscape* variables are initialized there, and only valid
(ie Index of 1 or 2)
index values are processed once in INPUT_STATE_LEFTOPENBRACKET_2.
I can add some comments to this effect if you think it is worthwhile.
Post by Ard Biesheuvel
Post by Roy Franz
TimerStatus = gBS->CheckEvent (TerminalDevice->TwoSecondTimeOut);
@@ -1223,7 +1225,8 @@ UnicodeToEfiKey (
continue;
}
- if (UnicodeChar == 'O' && TerminalDevice->TerminalType == VT100TYPE) {
+ if (UnicodeChar == 'O' && (TerminalDevice->TerminalType == VT100TYPE ||
+ TerminalDevice->TerminalType == TTYTERMTYPE)) {
TerminalDevice->InputState |= INPUT_STATE_O;
TerminalDevice->ResetState = RESET_STATE_DEFAULT;
continue;
@@ -1371,6 +1374,22 @@ UnicodeToEfiKey (
break;
}
+ } else if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+ /* Also accept VT100 escape codes for F1-F4 for TTY term */
+ switch (UnicodeChar) {
+ Key.ScanCode = SCAN_F1;
+ break;
+ Key.ScanCode = SCAN_F2;
+ break;
+ Key.ScanCode = SCAN_F3;
+ break;
+ Key.ScanCode = SCAN_F4;
+ break;
+ }
}
if (Key.ScanCode != SCAN_NULL) {
@@ -1514,6 +1533,21 @@ UnicodeToEfiKey (
}
}
+ /*
+ * The VT220 escape codes that the TTY terminal accepts all have
+ * numeric codes, and there are no ambiguous prefixes shared with
+ * other terminal types.
+ */
+ if (TerminalDevice->TerminalType == TTYTERMTYPE &&
+ Key.ScanCode == SCAN_NULL &&
+ UnicodeChar >= '0' &&
+ UnicodeChar <= '9') {
+ TtyEscapeStr[0] = UnicodeChar;
+ TtyEscapeIndex = 1;
+ TerminalDevice->InputState |= INPUT_STATE_LEFTOPENBRACKET_2;
+ continue;
+ }
+
if (Key.ScanCode != SCAN_NULL) {
Key.UnicodeChar = 0;
EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
@@ -1527,6 +1561,65 @@ UnicodeToEfiKey (
break;
+ /*
+ * Here we handle the VT220 escape codes that we accept. This
+ * state is only used by the TTY terminal type.
+ */
+ Key.ScanCode = SCAN_NULL;
+ if (TerminalDevice->TerminalType == TTYTERMTYPE) {
+
+ if (UnicodeChar == '~' && TtyEscapeIndex <= 2) {
+ UINTN EscCode;
+ TtyEscapeStr[TtyEscapeIndex] = 0; /* Terminate string */
+ EscCode = StrDecimalToUintn(TtyEscapeStr);
+ switch (EscCode) {
+ Key.ScanCode = SCAN_DELETE;
+ break;
+ Key.ScanCode = SCAN_F1 + EscCode - 11;
+ break;
+ Key.ScanCode = SCAN_F6 + EscCode - 17;
+ break;
+ Key.ScanCode = SCAN_F11 + EscCode - 23;
+ break;
+ break;
+ }
+ } else if (TtyEscapeIndex == 1){
+ /* 2 character escape code */
+ TtyEscapeStr[TtyEscapeIndex++] = UnicodeChar;
+ continue;
+ }
+ else {
+ DEBUG ((EFI_D_ERROR, "Unexpected state in escape2\n"));
+ }
+ }
+ TerminalDevice->ResetState = RESET_STATE_DEFAULT;
+
+ if (Key.ScanCode != SCAN_NULL) {
+ Key.UnicodeChar = 0;
+ EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
+ TerminalDevice->InputState = INPUT_STATE_DEFAULT;
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ continue;
+ }
+
+ UnicodeToEfiKeyFlushState (TerminalDevice);
+ break;
+
//
// Invalid state. This should never happen.
Changing state machine code like this makes me nervous, but as far as
I can tell, non-TtyTerm users are not affected by it, so there is
little risk of regression.
Roy Franz
2015-07-06 23:04:39 UTC
Permalink
From: Laszlo Ersek <***@redhat.com>

Add a fixed pointer PCD to allow build-time selection of VT100 or TTY terminal
type. The default remains VT100 emulation.
Add support for building the ARM QEMU platforms with the TTY terminal
with the "-D TTY_TERMINAL" build option.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <***@redhat.com>
[Roy Franz: minor edits: add TtyTerminal GUID, rename LINUX_TERMINAL to TTY_TERMINAL]
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Roy Franz <***@linaro.org>
---
ArmVirtPkg/ArmVirt.dsc.inc | 6 ++++++
ArmVirtPkg/ArmVirtPkg.dec | 7 +++++++
ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c | 12 ++++++++----
ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf | 4 ++++
4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 7ec0de4..2feebd3 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -15,6 +15,7 @@

[Defines]
DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
+ DEFINE TTY_TERMINAL = FALSE

[LibraryClasses.common]
!if $(TARGET) == RELEASE
@@ -359,6 +360,11 @@
gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
!endif

+!if $(TTY_TERMINAL) == TRUE
+ # Set terminal type to TtyTerm, the value encoded is EFI_TTY_TERM_GUID
+ gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x80, 0x6d, 0x91, 0x7d, 0xb1, 0x5b, 0x8c, 0x45, 0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94}
+!endif
+
[Components.common]
#
# Networking stack
diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index 7bbd9ff..9833c5a 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -49,6 +49,13 @@
#
gArmVirtTokenSpaceGuid.PcdDeviceTreeAllocationPadding|256|UINT32|0x00000002

+ #
+ # Binary representation of the GUID that determines the terminal type. The
+ # size must be exactly 16 bytes. The default value corresponds to
+ # EFI_VT_100_GUID.
+ #
+ gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
+
[PcdsDynamic, PcdsFixedAtBuild]
#
# ARM PSCI function invocations can be done either through hypervisor
diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
index 13830cb..b242a29 100644
--- a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
+++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
@@ -35,7 +35,7 @@
typedef struct {
VENDOR_DEVICE_PATH SerialDxe;
UART_DEVICE_PATH Uart;
- VENDOR_DEFINED_DEVICE_PATH Vt100;
+ VENDOR_DEFINED_DEVICE_PATH TermType;
EFI_DEVICE_PATH_PROTOCOL End;
} PLATFORM_SERIAL_CONSOLE;
#pragma pack ()
@@ -67,14 +67,16 @@ STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
},

//
- // VENDOR_DEFINED_DEVICE_PATH Vt100
+ // VENDOR_DEFINED_DEVICE_PATH TermType
//
{
{
MESSAGING_DEVICE_PATH, MSG_VENDOR_DP,
DP_NODE_LEN (VENDOR_DEFINED_DEVICE_PATH)
- },
- EFI_VT_100_GUID
+ }
+ //
+ // Guid to be filled in dynamically
+ //
},

//
@@ -421,6 +423,8 @@ PlatformBdsPolicyBehavior (
//
// Add the hardcoded serial console device path to ConIn, ConOut, ErrOut.
//
+ CopyGuid (&mSerialConsole.TermType.Guid,
+ PcdGetPtr (PcdTerminalTypeGuidBuffer));
BdsLibUpdateConsoleVariable (L"ConIn",
(EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
BdsLibUpdateConsoleVariable (L"ConOut",
diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
index d998216..9a3cfcd 100644
--- a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
+++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
@@ -39,6 +39,7 @@
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
+ ArmVirtPkg/ArmVirtPkg.dec

[LibraryClasses]
BaseLib
@@ -61,6 +62,9 @@
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits

+[Pcd]
+ gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer
+
[Guids]
gEfiFileInfoGuid
gEfiFileSystemInfoGuid
--
2.1.4
Laszlo Ersek
2015-07-07 13:01:04 UTC
Permalink
Ard,
Post by Roy Franz
Add a fixed pointer PCD to allow build-time selection of VT100 or TTY terminal
type. The default remains VT100 emulation.
Add support for building the ARM QEMU platforms with the TTY terminal
with the "-D TTY_TERMINAL" build option.
Contributed-under: TianoCore Contribution Agreement 1.0
[Roy Franz: minor edits: add TtyTerminal GUID, rename LINUX_TERMINAL to TTY_TERMINAL]
Contributed-under: TianoCore Contribution Agreement 1.0
---
ArmVirtPkg/ArmVirt.dsc.inc | 6 ++++++
ArmVirtPkg/ArmVirtPkg.dec | 7 +++++++
ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c | 12 ++++++++----
ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf | 4 ++++
4 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 7ec0de4..2feebd3 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -15,6 +15,7 @@
[Defines]
DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
+ DEFINE TTY_TERMINAL = FALSE
[LibraryClasses.common]
!if $(TARGET) == RELEASE
@@ -359,6 +360,11 @@
gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
!endif
+!if $(TTY_TERMINAL) == TRUE
+ # Set terminal type to TtyTerm, the value encoded is EFI_TTY_TERM_GUID
+ gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x80, 0x6d, 0x91, 0x7d, 0xb1, 0x5b, 0x8c, 0x45, 0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94}
+!endif
+
[Components.common]
#
# Networking stack
diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index 7bbd9ff..9833c5a 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -49,6 +49,13 @@
#
gArmVirtTokenSpaceGuid.PcdDeviceTreeAllocationPadding|256|UINT32|0x00000002
+ #
+ # Binary representation of the GUID that determines the terminal type. The
+ # size must be exactly 16 bytes. The default value corresponds to
+ # EFI_VT_100_GUID.
+ #
+ gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
+
[PcdsDynamic, PcdsFixedAtBuild]
#
# ARM PSCI function invocations can be done either through hypervisor
diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
index 13830cb..b242a29 100644
--- a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
+++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
@@ -35,7 +35,7 @@
typedef struct {
VENDOR_DEVICE_PATH SerialDxe;
UART_DEVICE_PATH Uart;
- VENDOR_DEFINED_DEVICE_PATH Vt100;
+ VENDOR_DEFINED_DEVICE_PATH TermType;
EFI_DEVICE_PATH_PROTOCOL End;
} PLATFORM_SERIAL_CONSOLE;
#pragma pack ()
@@ -67,14 +67,16 @@ STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
},
//
- // VENDOR_DEFINED_DEVICE_PATH Vt100
+ // VENDOR_DEFINED_DEVICE_PATH TermType
//
{
{
MESSAGING_DEVICE_PATH, MSG_VENDOR_DP,
DP_NODE_LEN (VENDOR_DEFINED_DEVICE_PATH)
- },
- EFI_VT_100_GUID
+ }
+ //
+ // Guid to be filled in dynamically
+ //
},
//
@@ -421,6 +423,8 @@ PlatformBdsPolicyBehavior (
//
// Add the hardcoded serial console device path to ConIn, ConOut, ErrOut.
//
+ CopyGuid (&mSerialConsole.TermType.Guid,
+ PcdGetPtr (PcdTerminalTypeGuidBuffer));
BdsLibUpdateConsoleVariable (L"ConIn",
(EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
BdsLibUpdateConsoleVariable (L"ConOut",
diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
index d998216..9a3cfcd 100644
--- a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
+++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
@@ -39,6 +39,7 @@
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
+ ArmVirtPkg/ArmVirtPkg.dec
[LibraryClasses]
BaseLib
@@ -61,6 +62,9 @@
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
+[Pcd]
+ gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer
+
[Guids]
gEfiFileInfoGuid
gEfiFileSystemInfoGuid
can you please review this? I can't ACK it because I co-authored it with
Roy.

Thanks
Laszlo
Ard Biesheuvel
2015-07-07 13:05:06 UTC
Permalink
Post by Roy Franz
Add a fixed pointer PCD to allow build-time selection of VT100 or TTY terminal
type. The default remains VT100 emulation.
Add support for building the ARM QEMU platforms with the TTY terminal
with the "-D TTY_TERMINAL" build option.
Contributed-under: TianoCore Contribution Agreement 1.0
[Roy Franz: minor edits: add TtyTerminal GUID, rename LINUX_TERMINAL to TTY_TERMINAL]
Contributed-under: TianoCore Contribution Agreement 1.0
---
ArmVirtPkg/ArmVirt.dsc.inc | 6 ++++++
ArmVirtPkg/ArmVirtPkg.dec | 7 +++++++
ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c | 12 ++++++++----
ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf | 4 ++++
4 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 7ec0de4..2feebd3 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -15,6 +15,7 @@
[Defines]
DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
+ DEFINE TTY_TERMINAL = FALSE
[LibraryClasses.common]
!if $(TARGET) == RELEASE
@@ -359,6 +360,11 @@
gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
!endif
+!if $(TTY_TERMINAL) == TRUE
+ # Set terminal type to TtyTerm, the value encoded is EFI_TTY_TERM_GUID
+ gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x80, 0x6d, 0x91, 0x7d, 0xb1, 0x5b, 0x8c, 0x45, 0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94}
+!endif
+
[Components.common]
#
# Networking stack
diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index 7bbd9ff..9833c5a 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -49,6 +49,13 @@
#
gArmVirtTokenSpaceGuid.PcdDeviceTreeAllocationPadding|256|UINT32|0x00000002
+ #
+ # Binary representation of the GUID that determines the terminal type. The
+ # size must be exactly 16 bytes. The default value corresponds to
+ # EFI_VT_100_GUID.
+ #
+ gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
+
[PcdsDynamic, PcdsFixedAtBuild]
#
# ARM PSCI function invocations can be done either through hypervisor
diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
index 13830cb..b242a29 100644
--- a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
+++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
@@ -35,7 +35,7 @@
typedef struct {
VENDOR_DEVICE_PATH SerialDxe;
UART_DEVICE_PATH Uart;
- VENDOR_DEFINED_DEVICE_PATH Vt100;
+ VENDOR_DEFINED_DEVICE_PATH TermType;
EFI_DEVICE_PATH_PROTOCOL End;
} PLATFORM_SERIAL_CONSOLE;
#pragma pack ()
@@ -67,14 +67,16 @@ STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
},
//
- // VENDOR_DEFINED_DEVICE_PATH Vt100
+ // VENDOR_DEFINED_DEVICE_PATH TermType
//
{
{
MESSAGING_DEVICE_PATH, MSG_VENDOR_DP,
DP_NODE_LEN (VENDOR_DEFINED_DEVICE_PATH)
- },
- EFI_VT_100_GUID
+ }
+ //
+ // Guid to be filled in dynamically
+ //
},
//
@@ -421,6 +423,8 @@ PlatformBdsPolicyBehavior (
//
// Add the hardcoded serial console device path to ConIn, ConOut, ErrOut.
//
+ CopyGuid (&mSerialConsole.TermType.Guid,
+ PcdGetPtr (PcdTerminalTypeGuidBuffer));
BdsLibUpdateConsoleVariable (L"ConIn",
(EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
BdsLibUpdateConsoleVariable (L"ConOut",
diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
index d998216..9a3cfcd 100644
--- a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
+++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
@@ -39,6 +39,7 @@
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
+ ArmVirtPkg/ArmVirtPkg.dec
[LibraryClasses]
BaseLib
@@ -61,6 +62,9 @@
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
+[Pcd]
+ gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer
+
[Guids]
gEfiFileInfoGuid
gEfiFileSystemInfoGuid
--
2.1.4
Loading...