Presently, VirtioInputDxe accepts key notifications (RegisterKeyNotifyEx)
but does not act upon them. Despite ConSplitter correctly registering
notifications via all input devices (including PS/2 etc. devices
that implement key notifications correctly), emulators such as QEMU
stop sending input to non-virtio keyboards as soon as the virtio device
is claimed. As a result, preboot hotkeys are non-functional when virtio
is used.
Fixes: https://github.com/tianocore/edk2/issues/12887
Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
Tested-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
In addition to maintaining full key state, we also need to maintain
explicit toggle (NumLock/CapsLock/ScrollLock) state. This completes
->KeyState->KeyToggleState handling and SetStateEx() implementation.
When a *Lock key is pressed, update the keyboard toggle state before
doing anything else. This must happen first, as this state is copied
into ->KeyState (and ->KeyState needs to be filled before ->Key, as
Shift state must be cleared after shifting a printable character).
When filling ->Key, apply CapsLock in series with Shift key handling
(CapsLock only applies to alphabetic characters, but otherwise it XORs
with Shift).
Finally, complete SetStateEx() implementation as we can now apply all
KeyToggleState bits properly.
Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
Tested-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
Folding Ctrl into a printable character to produce a C0 control code is
not specified by SIMPLE_TEXT_INPUT(_EX). We apply it for narrow
compatibility with other keyboard drivers and TerminalDxe; as such,
only apply it ReadKeyStroke (which is the only interface that does not
convey modifier state explicitly).
Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
Tested-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
After applying Shift to a printable character, we must clear
`EFI_*_SHIFT_PRESSED` from ->KeyState to follow SIMPLE_TEXT_INPUT(_EX).
This means that ->KeyState must be populated before key code is parsed.
Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
Tested-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
WaitForKey and ReadKeyStroke of SIMPLE_TEXT_INPUT must omit partial
keystrokes, as they do not return modifier and shift state (thus a
partial keystroke would read as an empty struct). Skip over partial
keystrokes in the key queue, should there be any.
Note that WaitForKeyEx of SIMPLE_TEXT_INPUT_EX must also omit partials,
thus it continues to share the implementation with WaitForKey -- unlike
ReadKeyStrokeEx and ReadKeyStroke.
Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
Tested-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
With support for partial keystrokes, the one-element "last key" buffer
is now actively lossy, as a single compound key press (such as Shift-S)
will produce two EFI_KEY_DATAs which will likely be processed in the
same GetDeviceData() loop, but only one of them would be retained.
Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
Tested-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
- on each event, compute and save full key state at the time of event
rather than just the input key itself
- correctly handle unrepresentable events (events which result in a
null EFI scancode / Unicode value) and do not enqueue them as keys
unless EFI_KEY_STATE_EXPOSED
- on ReadKeyStrokeEx, return the saved full key state rather than
recompute modifier state based on potentially-changed data
- on SetStateEx, update EFI_KEY_STATE_EXPOSED
(toggle state is not implemented yet)
Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
Tested-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
EFI_INPUT_KEY's ->ScanCode and ->UnicodeChar are mutually exclusive.
The value space of ->ScanCode only includes representations of keys that
are not representable in Unicode (F1-F10, Esc, arrows etc.) and vice
versa. Furthermore, the integer representations of those fields overlap.
As such, writing ->UnicodeChar values into ->ScanCode is invalid and
produces bogus EFI_INPUT_KEYs that would compare inequal to any
well-formed values that might be passed into e.g. RegisterKeyNotifyEx.
Do not set ->ScanCode when a Unicode value is produced for a printable
character.
Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
Tested-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
Technically, beyond KEY_PRESSED and its inverse KEY_RELEASED, a third
type of event is possible for autorepeated keystrokes. Invert the
condition on Event->Value to handle autorepeat events as (successive)
key presses rather than repeated releases (essentially ignoring them).
Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
Tested-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
The original implementation of AcpiAmlObjectUpdateInteger had a critical
bug when updating integer objects that were encoded with AML_ZERO_OP(0x00)
or AML_ONE_OP(0x01), which are 1-byte optimized encodings.
When the caller tried to update such an object to a value other than 0
or 1, the code would:
1. Overwrite the opcode byte with the new value's LSB
2. This changed the opcode itself, e.g. 0x0B becomes AML_WORD_PREFIX
3. Subsequent AML bytes (name segments of following objects) get
misinterpreted as integer data
4. Result: silent AML structure is silently corrupted, causing the OS to
fail parsing ACPI tables and eventually crash.
The fix:
1. Only allow 0 -> 0 or 1 updates using the original 1-byte encoding
2. For any other value, explicitly fail with a diagnostic
3. Provide clear debug instructions on how to fix the ASL source
Reviewed-by: jie.fu <jie.fu@cixtech.com>
Signed-off-by: Johnny.Fan <Johnny.Fan@cixtech.com>
I've missed that this handle should be my GitHub handle. Correct it to point
contributors to the right GitHub account.
Fixes: d795fb571b ("Maintainer.txt: add myself as reviewer for bhyve's OvmfPkg")
Signed-off-by: Corvin Köhne <corvink@FreeBSD.org>
12828 introduced an ASSERT in HiiGetBrowserData() that fires when
InternalHiiBrowserCallback() returns NULL. This is a valid return
value indicating the browser has no data for the requested variable,
and callers already handle this by checking the FALSE return value.
The ASSERT is incorrect because it triggers on a non-error path,
causing a crash when the browser callback legitimately returns no data.
Remove the unnecessary ASSERT while keeping the existing FALSE return
so callers continue to handle this case gracefully.
Cc: Qihang Gao <gaoqihang@loongson.cn>
Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
Performs a check that will verify that the library instance implements
the library specified in the dsc by ensuring a LIBRARY_CLASS definition
exists in the INF [Defines] section and the value matches the library it
says it is implementing.
As an example, from a platform dsc file:
BaseBmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
BaseBmpSupportLib is supposed to be of library class BmpSupportLib, but the
dsc defines it incorrectly, the warning message will be displayed during
build.
Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
Co-authored-by: Poncho Figueroa <poncho.figueroa.esqueda@intel.com>
Updates the fork repo to the TianoCore Uncrustify fork at:
https://github.com/tianocore/uncrustify
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Runs the Black formatter against UncrustifyCheck.py so is formatted
to PEP-8. No functional changes are made.
https://pypi.org/project/black/
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
The plugin previously assumed that the workspace was a git repository
and made that pacakges would largely reside in that same repository.
A few changes are made to better support multi-repo workspaces:
1. Added a new method to find the git repo that contains the package
being checked: `_get_git_repo_path()`.
2. Removes exceptions on git not being present and ignore/submodule
exceptions.
3. Checks for git ignored files (and similar) in the repo containing
the package being checked in `_get_git_ignored_paths()`.
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
https://codeql.github.com/codeql-query-help/cpp/cpp-comparison-with-wider-type
If the narrow type (smaller range) is compared against a wide type
(larger range), the narrow value may overflow before reaching the wide
value. This can cause unexpected behavior, such as:
Infinite loops (loop condition never becomes false).
Incorrect logic (comparison results are misleading).
Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
https://github.com/github/codeql/blob/codeql-cli-2.7.3/cpp/ql/src/Security/CWE/CWE-457/ConditionallyUninitializedVariable.qhelp
Some local variables, when going through a code path, can
end up uninitialized (using the value they had at the start
of the function). This is generally due to an error path
that can occur based on the library instances, or the
unchecked error (i.e. a allocation failing).
These variables should be initialized with a known value
that will result in the function being able to exit
gracefully.
Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
https://codeql.github.com/codeql-query-help/cpp/cpp-comparison-with-wider-type
If the narrow type (smaller range) is compared against a wide type
(larger range), the narrow value may overflow before reaching the wide
value. This can cause unexpected behavior, such as:
Infinite loops (loop condition never becomes false).
Incorrect logic (comparison results are misleading).
Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
https://codeql.github.com/codeql-query-help/cpp/cpp-comparison-with-wider-type
If the narrow type (smaller range) is compared against a wide type
(larger range), the narrow value may overflow before reaching the wide
value. This can cause unexpected behavior, such as:
Infinite loops (loop condition never becomes false).
Incorrect logic (comparison results are misleading).
Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
Fix codeql reported issues by flattening MainCmdEfiDecompress(),
making it easier for the tool to evaluate potential risks.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Refactor the Comp command and extract 2 functions:
- OpenFileOperand()
- CompareFiles()
This allows to simplify the logic of MainCmdComp() and
fix some codeql reported potential errors.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Check the return value of LocateProtocol() before
using the decompression protocol. This avoids a
potential NULL pointer derefence spotted by codeql.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Extract file opening and compression code into helpers.
Upon calling:
- gEfiShellProtocol->GetFileSize()
- gEfiShellProtocol->ReadFile()
the returned Status is now checked.
Upon calling AllocateZeroPool, the failed status is now set to
EFI_OUT_OF_RESOURCES.
Other than that, no functional change.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
This patch aims to help breaking down the long functions present in
the ShellPkg and reduce complexity/nested code and conditions.
Lower the indentation level in the newly created MainCmdXXX()
functions.
To avoid having one large commit updating all the UefiShellDebug1
commands, only update these files:
- SerMode.c
- SetSize.c
- SetVar.c
- SmbiosView/SmbiosView.c
No functional change should be induced by this patch.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
This patch aims to help breaking down the long functions present in
the ShellPkg and reduce complexity/nested code and conditions.
Lower the indentation level in the newly created MainCmdXXX()
functions.
To avoid having one large commit updating all the UefiShellDebug1
commands, only update these files:
- MemMap.c
- Mm.c
- Mode.c
- Pci.c
No functional change should be induced by this patch.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
This patch aims to help breaking down the long functions present in
the ShellPkg and reduce complexity/nested code and conditions.
Lower the indentation level in the newly created MainCmdXXX()
functions.
To avoid having one large commit updating all the UefiShellDebug1
commands, only update these files:
- Edit/Edit.c
- EfiCompress.c
- EfiDecompress.c
- LoadPciRom.c
No functional change should be induced by this patch.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
This patch aims to help breaking down the long functions present in
the ShellPkg and reduce complexity/nested code and conditions.
Lower the indentation level in the newly created MainCmdXXX()
functions.
To avoid having one large commit updating all the UefiShellDebug1
commands, only update these files:
- Comp.c
- Cxl.c
- Dblk.c
- Dmem.c
- DmpStore.c
No functional change should be induced by this patch.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
This patch aims to help breaking down the long functions present in
the ShellPkg and reduce complexity/nested code and conditions.
Extract a MainCmdXXX() function for each shell command.
This command contains the possible operations the command aims
to operate. The ShellCommandRunXXX() function from which it
is extracted is only responsible of:
- initializing the shell/command environment
- parsing the command parameter and creating a Package
- freeing the Package
For the MemMap and SetVar commands, ShellCommandLineFreeVarList()
calls are removed as the Package is now freed in the caller
function: ShellCommandRunXXX().
To avoid having one large commit updating all the UefiShellDebug1
commands, only update these files:
- SerMode.c
- SetSize.c
- SetVar.c
- SmbiosView/SmbiosView.c
No functional change should be induced by this patch.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
This patch aims to help breaking down the long functions present in
the ShellPkg and reduce complexity/nested code and conditions.
Extract a MainCmdXXX() function for each shell command.
This command contains the possible operations the command aims
to operate. The ShellCommandRunXXX() function from which it
is extracted is only responsible of:
- initializing the shell/command environment
- parsing the command parameter and creating a Package
- freeing the Package
For the MemMap and SetVar commands, ShellCommandLineFreeVarList()
calls are removed as the Package is now freed in the caller
function: ShellCommandRunXXX().
To avoid having one large commit updating all the UefiShellDebug1
commands, only update these files:
- MemMap.c
- Mm.c
- Mode.c
- Pci.c
No functional change should be induced by this patch.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
This patch aims to help breaking down the long functions present in
the ShellPkg and reduce complexity/nested code and conditions.
Extract a MainCmdXXX() function for each shell command.
This command contains the possible operations the command aims
to operate. The ShellCommandRunXXX() function from which it
is extracted is only responsible of:
- initializing the shell/command environment
- parsing the command parameter and creating a Package
- freeing the Package
For the MemMap and SetVar commands, ShellCommandLineFreeVarList()
calls are removed as the Package is now freed in the caller
function: ShellCommandRunXXX().
To avoid having one large commit updating all the UefiShellDebug1
commands, only update these files:
- Edit/Edit.c
- EfiCompress.c
- EfiDecompress.c
- HexEdit/HexEdit.c
- LoadPciRom.c
No functional change should be induced by this patch.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
This patch aims to help breaking down the long functions present in
the ShellPkg and reduce complexity/nested code and conditions.
Extract a MainCmdXXX() function for each shell command.
This command contains the possible operations the command aims
to operate. The ShellCommandRunXXX() function from which it
is extracted is only responsible of:
- initializing the shell/command environment
- parsing the command parameter and creating a Package
- freeing the Package
For the MemMap and SetVar commands, ShellCommandLineFreeVarList()
calls are removed as the Package is now freed in the caller
function: ShellCommandRunXXX().
To avoid having one large commit updating all the UefiShellDebug1
commands, only update these files:
- Comp.c
- Cxl.c
- Dblk.c
- Dmem.c
- DmpStore.c
No functional change should be induced by this patch.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
If ShellCommandLineParse() fails, there is no need to free:
- InputStr
- Package
Remove the goto statement.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Package is sometimes initialized to NULL and only
freed if not NULL. Remove these as:
- Package is initialized in ShellCommandLineParse().
- If ShellCommandLineFreeVarList() is reached,
Package cannot be NULL.
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
