Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Rebase & FF] AdvLoggerPkg: Modify AdvancedLoggerMmCore Library #582

Merged

Conversation

VivianNK
Copy link
Contributor

@VivianNK VivianNK commented Sep 20, 2024

Description

Make the following changes:

  • Include PiMm.h for EFI_MM_SYSTEM_TABLE definition
  • Moved DEBUG statement that could previously dereference a NULL pointer.
  • Make 'initialized' a global variable to match other library implementations and to be accessible for unit testing.
  • Change __FUNCTION__ to __func__
  • Add "MmCore" to the debug messages for easier identification
  • Add Version check in ValidateInfoBlock. The signature tells us we have the right structure, the version may be different if this MMCore instance is sharing a log buffer with DXE that thinks it is different, etc.

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

How This Was Tested

CI and Google Test

Integration Instructions

N/A

@VivianNK VivianNK requested a review from os-d September 20, 2024 23:45
@VivianNK VivianNK changed the title AdvLoggerPkg: Modify AdvancedLoggerMmCore Library [Rebase & FF] AdvLoggerPkg: Modify AdvancedLoggerMmCore Library Sep 20, 2024
@VivianNK VivianNK requested a review from kuqin12 September 20, 2024 23:46
@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2024

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 11.33%. Comparing base (ab7c17d) to head (6d330a1).

Files with missing lines Patch % Lines
...brary/AdvancedLoggerLib/MmCore/AdvancedLoggerLib.c 0.00% 7 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           dev/202405     #582      +/-   ##
==============================================
- Coverage       11.33%   11.33%   -0.01%     
==============================================
  Files             145      145              
  Lines           21931    21934       +3     
  Branches         2529     2529              
==============================================
  Hits             2486     2486              
- Misses          19411    19414       +3     
  Partials           34       34              
Flag Coverage Δ
AdvLoggerPkg 5.61% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

VivianNK added a commit to VivianNK/mu_plus that referenced this pull request Sep 21, 2024
Waiting on PR microsoft#582
- Include PiMm.h for EFI_MM_SYSTEM_TABLE definition
- Moved DEBUG statement that could previously dereference a NULL pointer.
- Make 'initialized' a global variable for testing purposes.
- Change __FUNCTION__ to __func__
- Add "MmCore" to the debug messages for easier identification
- Include PiMm.h for EFI_MM_SYSTEM_TABLE definition
- Moved DEBUG statement that could previously dereference a NULL pointer.
- Make 'initialized' a global variable for testing purposes.
- Change __FUNCTION__ to __func__
- Add "MmCore" to the debug messages for easier identification
@VivianNK VivianNK force-pushed the personal/vnowkakeane/advloggermmcore branch from 48045e0 to 6d330a1 Compare September 24, 2024 18:09
@VivianNK VivianNK requested a review from cfernald September 24, 2024 18:09
@VivianNK VivianNK merged commit d253606 into microsoft:dev/202405 Sep 24, 2024
28 checks passed
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.

4 participants