Skip to content

Conversation

maheeraeron
Copy link
Contributor

@maheeraeron maheeraeron commented Oct 15, 2025

This PR adds logic to filter logs for EFI diagnostics. By default, the logs will still default to show only ERROR and WARNING. This change must come first before the next UEFI update to preserve all logs to the in-memory buffer.

I will add some output soon to show what the inspect output looks like when you query for the LogLevel

@maheeraeron maheeraeron changed the title [WIP] firmware_uefi: EFI Diagnostics filtering firmware_uefi: EFI Diagnostics filtering Oct 21, 2025
@maheeraeron maheeraeron marked this pull request as ready for review October 21, 2025 23:51
@maheeraeron maheeraeron requested a review from a team as a code owner October 21, 2025 23:51
@Copilot Copilot AI review requested due to automatic review settings October 21, 2025 23:51
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements filtering functionality for EFI diagnostics logs in firmware_uefi. The changes add a configurable log level parameter that defaults to ERROR and WARN only, but can be expanded to include INFO or all log levels, ensuring the in-memory buffer preserves all logs while controlling what gets processed.

Key Changes:

  • Introduces a new LogLevel type with three configuration options: Default (ERROR/WARN), Info (ERROR/WARN/INFO), and Full (all levels)
  • Threads the log level configuration through the entire stack from CLI arguments to the UEFI firmware device
  • Updates the diagnostics processor to filter logs based on the configured level

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
vm/devices/get/guest_emulation_transport/src/client.rs Adds efi_diagnostics_log_level field to JSON deserialization
vm/devices/get/guest_emulation_transport/src/api.rs Imports EfiDiagnosticsLogLevelType and adds field to platform settings
vm/devices/get/guest_emulation_device/src/test_utilities.rs Sets default log level in test utilities
vm/devices/get/guest_emulation_device/src/resolver.rs Converts between resource and protocol log level types
vm/devices/get/guest_emulation_device/src/lib.rs Adds log level to GuestConfig and passes it through device platform settings
vm/devices/get/get_resources/src/lib.rs Defines EfiDiagnosticsLogLevelType enum in resources
vm/devices/get/get_protocol/src/dps_json.rs Defines EfiDiagnosticsLogLevelType for JSON protocol
vm/devices/firmware/firmware_uefi/src/service/diagnostics/processor.rs Updates processing logic to filter logs based on configured level
vm/devices/firmware/firmware_uefi/src/service/diagnostics/mod.rs Implements LogLevel wrapper type with filtering logic and inspect support
vm/devices/firmware/firmware_uefi/src/lib.rs Exports LogLevel and adds it to UefiConfig
petri/src/vm/openvmm/construct.rs Uses default log level in test configurations
openvmm/openvmm_entry/src/ttrpc/mod.rs Sets default log level in ttrpc configuration
openvmm/openvmm_entry/src/lib.rs Converts CLI arguments to log level configuration
openvmm/openvmm_entry/src/cli_args.rs Adds CLI argument for EFI diagnostics log level
openvmm/hvlite_defs/src/config.rs Defines EfiDiagnosticsLogLevelType in config definitions
openvmm/hvlite_core/src/worker/dispatch.rs Converts config log level to firmware LogLevel
openhcl/underhill_core/src/worker.rs Maps protocol log level to firmware LogLevel

pub enum EfiDiagnosticsLogLevelType {
/// Default log level
#[default]
Default = 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these need numbers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps not, although the corresponding mars file on the host did so I thought we'd at least be consistent. They are in the same order anyway, should I remove them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, if we're not actually relying on them in any way we shouldn't have them.

@maheeraeron
Copy link
Contributor Author

Hmm... just tested with OpenHCL on Hyper-V now but changing the WMI property is not getting picked up properly:
image

} else {
UefiCommandSet::Aarch64
},
diagnostics_log_level: match dps.general.efi_diagnostics_log_level.0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a fan of this syntax... should i just use regular if-statements instead...?

@github-actions
Copy link

@github-actions
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants