From 3a9579a76c1e5687b7cd3b5ef495c409ae85ef4b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 17 Apr 2024 09:23:33 -0700 Subject: [PATCH 1/3] pip: bump regex from 2023.12.25 to 2024.4.16 (#470) Bumps [regex](https://github.com/mrabarnett/mrab-regex) from 2023.12.25 to 2024.4.16. Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- pip-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pip-requirements.txt b/pip-requirements.txt index 166ae0b5b8..cce8a5d868 100644 --- a/pip-requirements.txt +++ b/pip-requirements.txt @@ -15,5 +15,5 @@ edk2-pytool-library==0.21.5 edk2-pytool-extensions==0.27.3 antlr4-python3-runtime==4.13.1 -regex==2023.12.25 +regex==2024.4.16 pygount==1.6.1 From b748acbf5cfd3b2221d03cc528c13fcdc33e2fb5 Mon Sep 17 00:00:00 2001 From: kuqin12 <42554914+kuqin12@users.noreply.github.com> Date: Wed, 17 Apr 2024 13:28:10 -0700 Subject: [PATCH 2/3] Put log entries into HOBs in PEI during log buffer initialization (#466) # Preface Please ensure you have read the [contribution docs](https://github.com/microsoft/mu/blob/master/CONTRIBUTING.md) prior to submitting the pull request. In particular, [pull request guidelines](https://github.com/microsoft/mu/blob/master/CONTRIBUTING.md#pull-request-best-practices). ## Description The current implementation will directly allocation pages through PEI services when Advanced logger is not initialized. This cause an issue of permanent loop if/when there are prints from the allocation routine. This change will create a hob during this interim state and log messages to hobs temporarily and coalesce the hobs after the allocation is finalized. This will reduce the print restriction down to hob creation only. For each item, place an "x" in between `[` and `]` if true. Example: `[x]`. _(you can also check items in the GitHub UI)_ - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... ## How This Was Tested This is tested on QEMU SBSA as well as Q35. Both booted to UEFI shell. ## Integration Instructions N/A --- AdvLoggerPkg/AdvLoggerPkg.dec | 10 +++ .../PeiCore/AdvancedLoggerLib.c | 80 ++++++++++++++++--- .../PeiCore/AdvancedLoggerLib.inf | 2 + 3 files changed, 81 insertions(+), 11 deletions(-) diff --git a/AdvLoggerPkg/AdvLoggerPkg.dec b/AdvLoggerPkg/AdvLoggerPkg.dec index ffa1395255..770d0bc300 100644 --- a/AdvLoggerPkg/AdvLoggerPkg.dec +++ b/AdvLoggerPkg/AdvLoggerPkg.dec @@ -57,6 +57,16 @@ # gAdvancedLoggerPreDxeLogsGuid = { 0x751fc006, 0x5804, 0x440d, { 0x8b, 0x15, 0x61, 0x8c, 0xf6, 0x56, 0xae, 0x76 } } + ## GUID for specifying Advanced logger interim hob, which is used to indicate whether the logger in the + # interim phase of logger buffer being initialized. + # + gAdvancedLoggerInterimHobGuid = { 0x36587034, 0xfcde, 0x45b4, { 0x9a, 0x2b, 0xd5, 0xc4, 0xc1, 0x39, 0x96, 0x5a } } + + ## GUID for specifying Advanced logger interim buffer hob, which is used to carry interim log entries during the + # interim phase of being initialized. + # + gAdvancedLoggerInterimBufHobGuid = { 0x4c4f8c0b, 0xbe54, 0x49b3, { 0xba, 0x54, 0x33, 0xbf, 0x14, 0x47, 0x75, 0x24 } } + [Ppis] ## Advanced Logger Ppi - Communication from PEIM to PEI_CORE library implementation # diff --git a/AdvLoggerPkg/Library/AdvancedLoggerLib/PeiCore/AdvancedLoggerLib.c b/AdvLoggerPkg/Library/AdvancedLoggerLib/PeiCore/AdvancedLoggerLib.c index 38823c4830..1315bdb730 100644 --- a/AdvLoggerPkg/Library/AdvancedLoggerLib/PeiCore/AdvancedLoggerLib.c +++ b/AdvLoggerPkg/Library/AdvancedLoggerLib/PeiCore/AdvancedLoggerLib.c @@ -9,7 +9,10 @@ #include #include +#include #include +#include +#include /** Including the PeiMain.h from PeiCore in order to access the Platform Blob data member. @@ -440,16 +443,20 @@ AdvancedLoggerGetLoggerInfo ( VOID ) { - UINTN BufferSize; - EFI_HOB_GUID_TYPE *GuidHob; - PEI_CORE_INSTANCE *PeiCoreInstance; - ADVANCED_LOGGER_INFO *LoggerInfo; - ADVANCED_LOGGER_INFO *LoggerInfoSec; - ADVANCED_LOGGER_PTR *LogPtr; - EFI_PHYSICAL_ADDRESS NewLoggerInfo; - UINTN Pages; - CONST EFI_PEI_SERVICES **PeiServices; - EFI_STATUS Status; + UINTN BufferSize; + EFI_HOB_GUID_TYPE *GuidHob; + EFI_HOB_GUID_TYPE *GuidHobInterim; + EFI_HOB_GUID_TYPE *GuidHobInterimBuf; + PEI_CORE_INSTANCE *PeiCoreInstance; + ADVANCED_LOGGER_INFO *LoggerInfo; + ADVANCED_LOGGER_INFO *LoggerInfoSec; + ADVANCED_LOGGER_PTR *LogPtr; + EFI_PHYSICAL_ADDRESS NewLoggerInfo; + UINTN Pages; + CONST EFI_PEI_SERVICES **PeiServices; + EFI_STATUS Status; + EFI_MEMORY_TYPE Type; + ADVANCED_LOGGER_MESSAGE_ENTRY_V2 *LogEntry; // Try to do the minimum work at the start of this function as this // is called quite often. @@ -492,6 +499,40 @@ AdvancedLoggerGetLoggerInfo ( } } + GuidHobInterim = GetFirstGuidHob (&gAdvancedLoggerInterimHobGuid); + if (GuidHobInterim != NULL) { + // In the middle of initialization, save the log to the interim hobs + Status = PeiServicesCreateHob ( + EFI_HOB_TYPE_GUID_EXTENSION, + (UINT16)(sizeof (EFI_HOB_GUID_TYPE) + sizeof (ADVANCED_LOGGER_INFO) + ADVANCED_LOGGER_MAX_MESSAGE_SIZE), + (VOID **)&GuidHobInterimBuf + ); + if (EFI_ERROR (Status)) { + return NULL; + } + + LoggerInfo = (ADVANCED_LOGGER_INFO *)GET_GUID_HOB_DATA (GuidHobInterimBuf); + BufferSize = sizeof (ADVANCED_LOGGER_INFO) + ADVANCED_LOGGER_MAX_MESSAGE_SIZE; + ZeroMem ((VOID *)LoggerInfo, BufferSize); + LoggerInfo->Signature = ADVANCED_LOGGER_SIGNATURE; + LoggerInfo->Version = ADVANCED_LOGGER_VERSION; + LoggerInfo->LogBuffer = PA_FROM_PTR (LoggerInfo + 1); + LoggerInfo->LogBufferSize = (UINT32)(BufferSize - sizeof (ADVANCED_LOGGER_INFO)); + LoggerInfo->LogCurrent = LoggerInfo->LogBuffer; + LoggerInfo->HwPrintLevel = FixedPcdGet32 (PcdAdvancedLoggerHdwPortDebugPrintErrorLevel); + AdvancedLoggerHdwPortInitialize (); + CopyGuid (&GuidHobInterimBuf->Name, &gAdvancedLoggerInterimBufHobGuid); + LoggerInfo->HdwPortInitialized = TRUE; + return LoggerInfo; + } else { + Status = PeiServicesCreateHob ( + EFI_HOB_TYPE_GUID_EXTENSION, + (UINT16)(sizeof (EFI_HOB_GUID_TYPE)), + (VOID **)&GuidHobInterim + ); + CopyGuid (&GuidHobInterim->Name, &gAdvancedLoggerInterimHobGuid); + } + // // No Logger Info - this must be the time to allocate a new LoggerInfo and save // the pointer in the PeiCoreInstance. @@ -516,14 +557,17 @@ AdvancedLoggerGetLoggerInfo ( if (FeaturePcdGet (PcdAdvancedLoggerPeiInRAM)) { Pages = FixedPcdGet32 (PcdAdvancedLoggerPages); + Type = EfiReservedMemoryType; } else { Pages = FixedPcdGet32 (PcdAdvancedLoggerPreMemPages); + // This is to avoid the interim buffer being allocated to consume 64KB on ARM64 platforms. + Type = EfiBootServicesData; } BufferSize = EFI_PAGES_TO_SIZE (Pages); Status = PeiServicesAllocatePages ( - EfiReservedMemoryType, + Type, Pages, &NewLoggerInfo ); @@ -561,6 +605,20 @@ AdvancedLoggerGetLoggerInfo ( UpdateSecLoggerInfo (LoggerInfo); } + // + // Check to see if we have anything in the interim buffer + // + GuidHobInterimBuf = GetFirstGuidHob (&gAdvancedLoggerInterimBufHobGuid); + while (GuidHobInterimBuf != NULL) { + // + // If we have an interim buffer, copy it to the new buffer + // + LoggerInfo = (ADVANCED_LOGGER_INFO *)GET_GUID_HOB_DATA (GuidHobInterimBuf); + LogEntry = (ADVANCED_LOGGER_MESSAGE_ENTRY_V2 *)(UINTN)LoggerInfo->LogBuffer; + AdvancedLoggerMemoryLoggerWrite (LogEntry->DebugLevel, LogEntry->MessageText, LogEntry->MessageLen); + GuidHobInterimBuf = GetNextGuidHob (&gAdvancedLoggerInterimHobGuid, GuidHobInterimBuf); + } + // // Publish the Advanced Logger Ppi // diff --git a/AdvLoggerPkg/Library/AdvancedLoggerLib/PeiCore/AdvancedLoggerLib.inf b/AdvLoggerPkg/Library/AdvancedLoggerLib/PeiCore/AdvancedLoggerLib.inf index 9ba99c4f33..5348657e4f 100644 --- a/AdvLoggerPkg/Library/AdvancedLoggerLib/PeiCore/AdvancedLoggerLib.inf +++ b/AdvLoggerPkg/Library/AdvancedLoggerLib/PeiCore/AdvancedLoggerLib.inf @@ -46,6 +46,8 @@ [Guids] gAdvancedLoggerHobGuid gEfiFirmwareFileSystem2Guid + gAdvancedLoggerInterimHobGuid + gAdvancedLoggerInterimBufHobGuid [Ppis] gAdvancedLoggerPpiGuid ## PRODUCES From cd7ce3ed8ae3ab38cdff67df6b8379a343347d52 Mon Sep 17 00:00:00 2001 From: John Schock Date: Thu, 18 Apr 2024 13:42:21 -0700 Subject: [PATCH 3/3] Relax report length requirements (#455) ## Description This PR brings in two fixes to support a wider variety of devices, some of which might not be completely self-consistent: * Allow report lengths that don't match the report descriptor. Reports that are shorter than the Report Descriptor specifies will be processed for whatever fields are fully present. Reports that are longer than the Report descriptor specifies will simply ignore the extra bytes in the report. * Move away from using `signal_event` to force keyboard layout initialization, and install call the layout change routine directly. This avoids introducing sequencing issues based on the TPL that the keyboard is being initialized at, resulting in more deterministic behavior. - [x] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [x] Includes tests? - [ ] Includes documentation? ## How This Was Tested Rust unit tests updated to cover new functionality all pass. Functional testing on hardware with the changes also passes. ## Integration Instructions N/A --- HidPkg/UefiHidDxeV2/src/keyboard/mod.rs | 109 ++++++++++-------- .../src/keyboard/simple_text_in.rs | 2 + .../src/keyboard/simple_text_in_ex.rs | 5 + HidPkg/UefiHidDxeV2/src/pointer/mod.rs | 33 ++++-- 4 files changed, 93 insertions(+), 56 deletions(-) diff --git a/HidPkg/UefiHidDxeV2/src/keyboard/mod.rs b/HidPkg/UefiHidDxeV2/src/keyboard/mod.rs index 77ec5cf70c..81886b872c 100644 --- a/HidPkg/UefiHidDxeV2/src/keyboard/mod.rs +++ b/HidPkg/UefiHidDxeV2/src/keyboard/mod.rs @@ -33,7 +33,7 @@ use crate::{ keyboard::key_queue::OrdKeyData, }; -use rust_advanced_logger_dxe::{debugln, function, DEBUG_ERROR, DEBUG_WARN}; +use rust_advanced_logger_dxe::{debugln, function, DEBUG_ERROR, DEBUG_VERBOSE, DEBUG_WARN}; // usages supported by this module const KEYBOARD_MODIFIER_USAGE_MIN: u32 = 0x000700E0; @@ -389,8 +389,8 @@ impl KeyboardHidHandler { fn initialize_keyboard_layout(&mut self) -> Result<(), efi::Status> { self.install_layout_change_event()?; - //signal event to pick up any existing layout - self.boot_services.signal_event(self.layout_change_event); + //fake signal event to pick up any existing layout + on_layout_update(self.layout_change_event, self.layout_context as *mut c_void); //install a default layout if no layout is installed. if self.key_queue.get_layout().is_none() { @@ -555,7 +555,18 @@ impl HidReportReceiver for KeyboardHidHandler { if let Some(report_data) = self.input_reports.get(&report_id).cloned() { if report.len() != report_data.report_size { - break 'report_processing; + //Some devices report extra bytes in their reports. Warn about this, but try and process anyway. + debugln!( + DEBUG_VERBOSE, + "{:?}:{:?} unexpected report length for report_id: {:?}. expected {:?}, actual {:?}", + function!(), + line!(), + report_id, + report_data.report_size, + report.len() + ); + debugln!(DEBUG_VERBOSE, "report: {:x?}", report); + //break 'report_processing; } //reset currently active keys to empty set. @@ -828,8 +839,11 @@ mod test { boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_create_event_ex().returning(|_, _, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, _| efi::Status::SUCCESS); + boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); boot_services.expect_signal_event().returning(|_| efi::Status::SUCCESS); boot_services.expect_open_protocol().returning(|_, _, _, _, _, _| efi::Status::NOT_FOUND); + boot_services.expect_raise_tpl().returning(|_| efi::TPL_APPLICATION); + boot_services.expect_restore_tpl().returning(|_| ()); let mut keyboard_handler = KeyboardHidHandler::new(boot_services, 1 as efi::Handle); let mut hid_io = MockHidIo::new(); @@ -848,6 +862,7 @@ mod test { boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_create_event_ex().returning(|_, _, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, _| efi::Status::SUCCESS); + boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); boot_services.expect_signal_event().returning(|_| efi::Status::SUCCESS); boot_services.expect_open_protocol().returning(|_, _, _, _, _, _| efi::Status::NOT_FOUND); boot_services.expect_raise_tpl().returning(|_| efi::TPL_APPLICATION); @@ -981,6 +996,47 @@ mod test { efi::Status::SUCCESS } + const TEST_KEYBOARD_GUID: efi::Guid = + efi::Guid::from_fields(0xf1796c10, 0xdafb, 0x4989, 0xa0, 0x82, &[0x75, 0xe9, 0x65, 0x76, 0xbe, 0x52]); + + static mut TEST_KEYBOARD_LAYOUT: HiiKeyboardLayout = + HiiKeyboardLayout { keys: Vec::new(), guid: TEST_KEYBOARD_GUID, descriptions: Vec::new() }; + unsafe { + //make a test keyboard layout that is different than the default. + TEST_KEYBOARD_LAYOUT = hii_keyboard_layout::get_default_keyboard_layout(); + TEST_KEYBOARD_LAYOUT.guid = TEST_KEYBOARD_GUID; + TEST_KEYBOARD_LAYOUT.keys.pop(); + TEST_KEYBOARD_LAYOUT.keys.pop(); + TEST_KEYBOARD_LAYOUT.keys.pop(); + TEST_KEYBOARD_LAYOUT.descriptions[0].description = "Test Keyboard Layout".to_string(); + TEST_KEYBOARD_LAYOUT.descriptions[0].language = "ts-TS".to_string(); + } + + extern "efiapi" fn get_keyboard_layout( + _this: *const protocols::hii_database::Protocol, + _key_guid: *const efi::Guid, + keyboard_layout_length: *mut u16, + keyboard_layout_ptr: *mut protocols::hii_database::KeyboardLayout, + ) -> efi::Status { + let mut keyboard_layout_buffer = vec![0u8; 4096]; + let buffer_size = keyboard_layout_buffer.pwrite(unsafe { &TEST_KEYBOARD_LAYOUT }, 0).unwrap(); + keyboard_layout_buffer.resize(buffer_size, 0); + unsafe { + if keyboard_layout_length.read() < buffer_size as u16 { + keyboard_layout_length.write(buffer_size as u16); + return efi::Status::BUFFER_TOO_SMALL; + } else { + if keyboard_layout_ptr.is_null() { + panic!("bad keyboard pointer)"); + } + keyboard_layout_length.write(buffer_size as u16); + let slice = from_raw_parts_mut(keyboard_layout_ptr as *mut u8, buffer_size); + slice.copy_from_slice(&keyboard_layout_buffer); + return efi::Status::SUCCESS; + } + } + } + extern "efiapi" fn set_keyboard_layout( _this: *const protocols::hii_database::Protocol, _key_guid: *mut efi::Guid, @@ -990,47 +1046,6 @@ mod test { boot_services.expect_restore_tpl().returning(|_| ()); boot_services.expect_signal_event().returning(|_| efi::Status::SUCCESS); - const TEST_KEYBOARD_GUID: efi::Guid = - efi::Guid::from_fields(0xf1796c10, 0xdafb, 0x4989, 0xa0, 0x82, &[0x75, 0xe9, 0x65, 0x76, 0xbe, 0x52]); - - static mut TEST_KEYBOARD_LAYOUT: HiiKeyboardLayout = - HiiKeyboardLayout { keys: Vec::new(), guid: TEST_KEYBOARD_GUID, descriptions: Vec::new() }; - unsafe { - //make a test keyboard layout that is different than the default. - TEST_KEYBOARD_LAYOUT = hii_keyboard_layout::get_default_keyboard_layout(); - TEST_KEYBOARD_LAYOUT.guid = TEST_KEYBOARD_GUID; - TEST_KEYBOARD_LAYOUT.keys.pop(); - TEST_KEYBOARD_LAYOUT.keys.pop(); - TEST_KEYBOARD_LAYOUT.keys.pop(); - TEST_KEYBOARD_LAYOUT.descriptions[0].description = "Test Keyboard Layout".to_string(); - TEST_KEYBOARD_LAYOUT.descriptions[0].language = "ts-TS".to_string(); - } - - extern "efiapi" fn get_keyboard_layout( - _this: *const protocols::hii_database::Protocol, - _key_guid: *const efi::Guid, - keyboard_layout_length: *mut u16, - keyboard_layout_ptr: *mut protocols::hii_database::KeyboardLayout, - ) -> efi::Status { - let mut keyboard_layout_buffer = vec![0u8; 4096]; - let buffer_size = keyboard_layout_buffer.pwrite(unsafe { &TEST_KEYBOARD_LAYOUT }, 0).unwrap(); - keyboard_layout_buffer.resize(buffer_size, 0); - unsafe { - if keyboard_layout_length.read() < buffer_size as u16 { - keyboard_layout_length.write(buffer_size as u16); - return efi::Status::BUFFER_TOO_SMALL; - } else { - if keyboard_layout_ptr.is_null() { - panic!("bad keyboard pointer)"); - } - keyboard_layout_length.write(buffer_size as u16); - let slice = from_raw_parts_mut(keyboard_layout_ptr as *mut u8, buffer_size); - slice.copy_from_slice(&keyboard_layout_buffer); - return efi::Status::SUCCESS; - } - } - } - boot_services.expect_locate_protocol().returning(|protocol, _, interface| { unsafe { match *protocol { @@ -1062,6 +1077,7 @@ mod test { let mut hii_database = hii_database.assume_init(); hii_database.new_package_list = new_package_list; hii_database.set_keyboard_layout = set_keyboard_layout; + hii_database.get_keyboard_layout = get_keyboard_layout; interface.write(Box::into_raw(Box::new(hii_database)) as *mut c_void); } @@ -1089,6 +1105,7 @@ mod test { boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_create_event_ex().returning(|_, _, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, _| efi::Status::SUCCESS); + boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); boot_services.expect_signal_event().returning(|_| efi::Status::SUCCESS); boot_services.expect_open_protocol().returning(|_, _, _, _, _, _| efi::Status::NOT_FOUND); boot_services.expect_raise_tpl().returning(|_| efi::TPL_APPLICATION); @@ -1137,6 +1154,7 @@ mod test { boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_create_event_ex().returning(|_, _, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, _| efi::Status::SUCCESS); + boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); boot_services.expect_signal_event().returning(|_| efi::Status::SUCCESS); boot_services.expect_open_protocol().returning(|_, _, _, _, _, _| efi::Status::NOT_FOUND); boot_services.expect_raise_tpl().returning(|_| efi::TPL_APPLICATION); @@ -1200,6 +1218,7 @@ mod test { boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_create_event_ex().returning(|_, _, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, _| efi::Status::SUCCESS); + boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); boot_services.expect_signal_event().returning(|_| efi::Status::SUCCESS); boot_services.expect_open_protocol().returning(|_, _, _, _, _, _| efi::Status::NOT_FOUND); boot_services.expect_raise_tpl().returning(|_| efi::TPL_APPLICATION); diff --git a/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in.rs b/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in.rs index eacae9aeb5..ff0b10e1c5 100644 --- a/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in.rs +++ b/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in.rs @@ -461,6 +461,7 @@ mod test { boot_services.expect_create_event_ex().returning(|_, _, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_signal_event().returning(|_| efi::Status::SUCCESS); boot_services.expect_open_protocol().returning(|_, _, _, _, _, _| efi::Status::NOT_FOUND); + boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); let mut keyboard_handler = KeyboardHidHandler::new(boot_services, 1 as efi::Handle); @@ -586,6 +587,7 @@ mod test { // used in install boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); + boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); boot_services.expect_install_protocol_interface().returning(|_, protocol, _, interface| { if unsafe { *protocol } == protocols::simple_text_input::PROTOCOL_GUID { CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); diff --git a/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in_ex.rs b/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in_ex.rs index 771b46a4c5..6313bf4996 100644 --- a/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in_ex.rs +++ b/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in_ex.rs @@ -599,6 +599,7 @@ mod test { boot_services.expect_create_event_ex().returning(|_, _, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_signal_event().returning(|_| efi::Status::SUCCESS); boot_services.expect_open_protocol().returning(|_, _, _, _, _, _| efi::Status::NOT_FOUND); + boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); let mut keyboard_handler = KeyboardHidHandler::new(boot_services, 1 as efi::Handle); @@ -742,6 +743,7 @@ mod test { CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); efi::Status::SUCCESS }); + boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); // used in set state boot_services.expect_raise_tpl().returning(|_| efi::TPL_APPLICATION); @@ -827,6 +829,7 @@ mod test { CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); efi::Status::SUCCESS }); + boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); // used in read key stroke boot_services.expect_raise_tpl().returning(|_| efi::TPL_APPLICATION); @@ -967,6 +970,8 @@ mod test { } efi::Status::SUCCESS }); + boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); + boot_services.expect_create_event_ex().returning(|_, _, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_raise_tpl().returning(|_| efi::TPL_APPLICATION); boot_services.expect_restore_tpl().returning(|_| ()); diff --git a/HidPkg/UefiHidDxeV2/src/pointer/mod.rs b/HidPkg/UefiHidDxeV2/src/pointer/mod.rs index b8ca2309da..3a37ebd5cb 100644 --- a/HidPkg/UefiHidDxeV2/src/pointer/mod.rs +++ b/HidPkg/UefiHidDxeV2/src/pointer/mod.rs @@ -21,7 +21,7 @@ use hidparser::{ }; use r_efi::{efi, protocols}; -use rust_advanced_logger_dxe::{debugln, function, DEBUG_ERROR}; +use rust_advanced_logger_dxe::{debugln, function, DEBUG_ERROR, DEBUG_VERBOSE}; use crate::{ boot_services::UefiBootServices, @@ -262,7 +262,18 @@ impl HidReportReceiver for PointerHidHandler { if let Some(report_data) = self.input_reports.get(&report_id).cloned() { if report.len() != report_data.report_size { - break 'report_processing; + //Some devices report extra bytes in their reports. Warn about this, but try and process anyway. + debugln!( + DEBUG_VERBOSE, + "{:?}:{:?} unexpected report length for report_id: {:?}. expected {:?}, actual {:?}", + function!(), + line!(), + report_id, + report_data.report_size, + report.len() + ); + debugln!(DEBUG_VERBOSE, "report: {:x?}", report); + //break 'report_processing; } // hand the report data to the handler for each relevant field for field-specific processing. @@ -589,7 +600,7 @@ mod test { } #[test] - fn bad_reports_should_be_ignored() { + fn bad_reports_should_be_processed_with_best_effort() { let boot_services = create_fake_static_boot_service(); static mut ABS_PTR_INTERFACE: *mut c_void = core::ptr::null_mut(); @@ -650,19 +661,19 @@ mod test { pointer_handler.receive_report(report, &hid_io); assert_eq!(pointer_handler.current_state.active_buttons, 0); - assert_eq!(pointer_handler.current_state.current_x, CENTER); - assert_eq!(pointer_handler.current_state.current_y, CENTER); - assert_eq!(pointer_handler.current_state.current_z, 0); - assert_eq!(pointer_handler.state_changed, false); + assert_eq!(pointer_handler.current_state.current_x, 0); + assert_eq!(pointer_handler.current_state.current_y, 4); + assert_eq!(pointer_handler.current_state.current_z, 4); + assert_eq!(pointer_handler.state_changed, true); //report too short let report: &[u8] = &[0x00, 0x10, 0x00, 0x10, 0x00, 0x10]; pointer_handler.receive_report(report, &hid_io); assert_eq!(pointer_handler.current_state.active_buttons, 0); - assert_eq!(pointer_handler.current_state.current_x, CENTER); - assert_eq!(pointer_handler.current_state.current_y, CENTER); - assert_eq!(pointer_handler.current_state.current_z, 0); - assert_eq!(pointer_handler.state_changed, false); + assert_eq!(pointer_handler.current_state.current_x, 4); + assert_eq!(pointer_handler.current_state.current_y, 4); + assert_eq!(pointer_handler.current_state.current_z, 4); + assert_eq!(pointer_handler.state_changed, true); } }