[REBASE & FF] Add AdvLogger PRM Module #471
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
language:python
os-d
force-pushed
the
osde/rebase_advloggerprm
branch
from
fb66a9c to
e5f76c4
Compare
makubacki
reviewed
Apr 22, 2024
makubacki
reviewed
Apr 22, 2024
makubacki
reviewed
Apr 22, 2024
makubacki
reviewed
Apr 22, 2024
...erOsConnectorPrm/Library/AdvLoggerOsConnectorPrmConfigLib/AdvLoggerOsConnectorPrmConfigLib.c
Show resolved
Hide resolved
makubacki
approved these changes
Apr 22, 2024
kuqin12
approved these changes
Apr 22, 2024
os-d
force-pushed
the
osde/rebase_advloggerprm
branch
3 times, most recently
from
6f24439 to
ee20e98
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/202311 #471 +/- ##
==================================================
- Coverage 12.13% 10.86% -1.28%
==================================================
Files 110 142 +32
Lines 18991 22379 +3388
Branches 1756 1808 +52
==================================================
+ Hits 2305 2431 +126
- Misses 16654 19916 +3262
Partials 32 32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
os-d
force-pushed
the
osde/rebase_advloggerprm
branch
from
ee20e98 to
b9f978e
Compare
os-d
commented
Apr 27, 2024
os-d
force-pushed
the
osde/rebase_advloggerprm
branch
2 times, most recently
from
1989cd9 to
5511e4b
Compare
5 tasks
The V4 Adv Logger Info structure uses pointers for LogCurrent and
LogBuffer. These pointers are not required and have additional
safety overhead. Furthermore, the pointers do not support a
runtime logger in an ARM64 environment, as StMM requires the
pointers to be physical but the runtime logger requires the
pointers to be virtual.
The V5 structure moves these to two new fields, LogCurrentOffset
and LogBufferOffset, respectively, to avoid using pointers and
be able to support runtime logging on ARM64.
- [x] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [x] 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, ...
- [x] 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, ...
Tested on various physical and virtual platforms to confirm the
log is output as expected with the V5 structure.
Update to this commit and use the new version of DecodeUefiLog.py
to correctly decode a V5 Adv Logger Log.
os-d
force-pushed
the
osde/rebase_advloggerprm
branch
2 times, most recently
from
a674a7c to
2c836d4
Compare
This commit adds the Adv Logger PRM Module. It is an alternate
way to fetch the Adv Logger Log that does not require SMM/Secure World
access. It also is independently serviceable from the OS.
- [x] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [x] 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, ...
- [x] Includes tests?
- **Tests** - Does the change include any explicit test code?
- Examples: Unit tests, integration tests, robot tests, ...
- [x] 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, ...
Tested on various physical and virtual platforms to ensure the
log was the same between the GetVariable interface and the PRM
interface.
See the README update for integration instructions.
os-d
force-pushed
the
osde/rebase_advloggerprm
branch
from
2c836d4 to
29200dc
Compare
5 tasks
os-d
added a commit
that referenced
this pull request
May 13, 2024
## Description Fix an error made in the AdvLogger v5 PR (#471). This is the same fix (but for a different variable name which is why it was mixed on the last fix up) as #483. Usage of mHighAddress (and mMaxAddress) will be evaluated as part of issue #474. - [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 Tested on Q35, LineParserTestApp was failing because of this. ## Integration Instructions N/A for a platform. For the next Mu integration, this should be combined with the Advanced Logger v5 commit be9a3d2.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This adds a PRM Module to fetch the Advanced Logger Log. It includes a sample Windows drive to query the PRM module and get the log. It also updates the Advanced Logger structure to V5 to use offsets instead of pointers for LogCurrent and LogBuffer. It updates DecodeUefiLog.py to be able to read this V5 structure.
flow, or firmware?
validation improvement, ...
in build or boot behavior?
a function in a new library class in a pre-existing module, ...
outside direct code modifications (and comments)?
on an a separate Web page, ...
How This Was Tested
Tested on Q35 on mu_tiano_platforms by booting to Windows, running the sample driver to fetch the log, then decoding with DecodeUefiLog.py. Also tested on various physical platforms.
Integration Instructions
See README updates for integration instructions.