| Message ID | 1436223879-27956-4-git-send-email-roy.franz@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 7 July 2015 at 01:04, Roy Franz <roy.franz@linaro.org> wrote: > 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 <roy.franz@linaro.org> > Contributed-under: TianoCore Contribution Agreement 1.0 Same here ^^^ (and in patch 1/4) > --- > 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? > 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. 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 <ard.biesheuvel@linaro.org> ------------------------------------------------------------------------------ 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/
On Tue, Jul 7, 2015 at 6:18 AM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 7 July 2015 at 01:04, Roy Franz <roy.franz@linaro.org> wrote: >> 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 <roy.franz@linaro.org> >> 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. 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. > >> 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. > > 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 <ard.biesheuvel@linaro.org> ------------------------------------------------------------------------------ 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/
On 7 July 2015 at 22:07, Roy Franz <roy.franz@linaro.org> wrote: > On Tue, Jul 7, 2015 at 6:18 AM, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: >> On 7 July 2015 at 01:04, Roy Franz <roy.franz@linaro.org> wrote: >>> 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 <roy.franz@linaro.org> >>> 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. > 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. > > >> >>> 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. >> >> 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 <ard.biesheuvel@linaro.org> ------------------------------------------------------------------------------ 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/
On Tue, Jul 7, 2015 at 1:25 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 7 July 2015 at 22:07, Roy Franz <roy.franz@linaro.org> wrote: >> On Tue, Jul 7, 2015 at 6:18 AM, Ard Biesheuvel >> <ard.biesheuvel@linaro.org> wrote: >>> On 7 July 2015 at 01:04, Roy Franz <roy.franz@linaro.org> wrote: >>>> 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 <roy.franz@linaro.org> >>>> 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 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. >> 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. >> >> >>> >>>> 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. >>> >>> 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 <ard.biesheuvel@linaro.org> ------------------------------------------------------------------------------ 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/
On Tue, Jul 7, 2015 at 1:41 PM, Andrew Fish <afish@apple.com> wrote: > > On Jul 7, 2015, at 1:31 PM, Roy Franz <roy.franz@linaro.org> wrote: > > On Tue, Jul 7, 2015 at 1:25 PM, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: > > On 7 July 2015 at 22:07, Roy Franz <roy.franz@linaro.org> wrote: > > On Tue, Jul 7, 2015 at 6:18 AM, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: > > On 7 July 2015 at 01:04, Roy Franz <roy.franz@linaro.org> wrote: > > 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 <roy.franz@linaro.org> > 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. > > #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 > > Thanks, > > Andrew Fish > > 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. > > > > 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. > > > 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 <ard.biesheuvel@linaro.org> > > > ------------------------------------------------------------------------------ > 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 > edk2-devel@lists.sourceforge.net > 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 > edk2-devel@lists.sourceforge.net > 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/
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.
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 <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(-)