Skip to content

Conversation

@PaddyDengAmi
Copy link

@PaddyDengAmi PaddyDengAmi commented Jan 29, 2026

Description

With the PCD enabled, assert lib will print the ASSERT message to serial port in addition of AdvancedLogger

  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?

How This Was Tested

Tested on platform and disable normal debug print via AdvancedLogger, the ASSERT message still shows up in the serial port console.

Integration Instructions

N/A

With the PCD enabled, assert lib will print the ASSERT message to serial
port in addition of AdvancedLogger

Signed-off-by: PaddyDeng <[email protected]>
@apop5 apop5 requested review from kuqin12 and os-d January 29, 2026 17:34
@os-d
Copy link
Contributor

os-d commented Jan 29, 2026

I wouldn't add the PCD and just always print the assert message to serial. If we are hitting assertlib, the system is dead, may as well just always print it.

Copy link
Member

@makubacki makubacki left a comment

Choose a reason for hiding this comment

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

This needs clarification as to what you are trying to do and why existing mechanisms are not sufficient. Then we can determine how to handle this.

Another PCD at the DebugLib level in the abstraction stack to bypass calling AdvancedLoggerWrite() which calls AdvancedLoggerHdwPortWrite() which calls SerialPortWrite() (in the non-null instance of AdvancedLoggerHdwPortLib) is strange.

@kuqin12
Copy link
Contributor

kuqin12 commented Jan 29, 2026

This needs clarification as to what you are trying to do and why existing mechanisms are not sufficient. Then we can determine how to handle this.

Another PCD at the DebugLib level in the abstraction stack to bypass calling AdvancedLoggerWrite() which calls AdvancedLoggerHdwPortWrite() which calls SerialPortWrite() (in the non-null instance of AdvancedLoggerHdwPortLib) is strange.

Agreed. This should have handled in the AdvancedLoggerWrite call above. You might want to figure out why the prints are not going through the AdvancedLoggerHdwPortWrite instead. Maybe HdwPortDisabled in the header was set to FALSE, or the error level was set improperly.

@MarcChen46
Copy link
Contributor

MarcChen46 commented Jan 30, 2026

Thanks @makubacki @kuqin12 and @os-d for your comments.

Background:
In our custom AdvancedLoggerHdwPortWrite implementation, we have a runtime debug mode check that controls whether debug messages are printed to the serial console. By default, this is disabled.

However, we want to always print ASSERT-related messages to the serial console, regardless of the debug mode setting. This ensures that during normal validation or any formal use case, any ASSERT will be immediately visible to the validation team, allowing them to investigate and debug issues in a timely manner.

Since our BIOS is built as a release build, the system does not halt on ASSERT. This makes it even more critical that ASSERT messages are always output to the serial console so they are not silently missed.

Alternative approach considered:
We could implement the ASSERT detection logic directly in AdvancedLoggerHdwPortWrite by checking if the message matches the ASSERT message pattern (e.g., checking if the buffer starts with "ASSERT"). However, this approach has a maintenance concern: if the ASSERT message format changes in the future(it actually not changed for 10 years, not sure if it is a real concern), our pattern-matching logic would also need to be updated to remain in sync.

Please provide your comment and suggestion, thanks a lot.

@makubacki
Copy link
Member

Please provide your comment and suggestion, thanks a lot.

The AssertLib.c file in AdvLoggerPkg is not that complex. Can you just create your own instance? Then you can have whatever logic you want in there.

@MarcChen46
Copy link
Contributor

Thanks for the comment, we will implement our own library.

@PaddyDengAmi, please close abandon this PR, thanks.

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.

5 participants