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

MmSupervisorPkg: Add MmSupervisorRing3Performance #390

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

makubacki
Copy link
Member

Description

Closes #383

Adds a new MM_STANDALONE driver that is used to link against a
PerformanceLib core instance for MM user mode drivers.

An empty driver is reserved for this use case for the following
reasons:

  1. Make integration and debug obvious and easy by having a clearly
    named and designated driver for this purpose.
  2. To be among the first MM user mode drivers dispatched to install
    the performance protocol and make the performance service
    available to other user mode drivers as early as possible.
  3. To avoid linking directly to MmSupervisorRing3Broker. The MM
    Core performance library instance code has functionality that
    depends on MM services to be available. Since the Ring 3 Broker
    is reponsible for initializing MM services in user mode, it is
    complicated and error prone to attempt to defer using those
    services to install the performance protocol until syscall has
    been set up with the core and a proper gMmst global could be
    used by the common MM code. This is all much simpler and easier
    to integrate in a separate MM user mode driver.
  4. To avoid linking the library to a pre-existing MM module that
    may have an unknown number of depdencies in its DEPEX. Since
    the DEPEX on a given platform aggregates all of the dependencies
    in linked libraries, it is not safe to assume any given MM module
    not dedicated exclusively for this purpose will not have unrelated
    dependencies that unnecessarily prolong installation of the
    performance protocol.

Temporarily includes this commit:

TEMP - .pytool/CISettings.py: Use Mu Basecore Standalone MM Perf Branch

Temporarily use this branch for testing.

This commit will be reverted after the following Mu Basecore PR is
merged:

microsoft/mu_basecore#1268


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

How This Was Tested

  • Use MmSupervisorRing3Performance to retrieve MM performance records
    with the MM Supervisor on Q35.

Integration Instructions

  • Included in MmSupervisorPkg/Docs/PlatformIntegration/PlatformIntegrationSteps.md

@makubacki makubacki added the type:feature-request A new feature proposal label Feb 3, 2025
@makubacki makubacki requested a review from kuqin12 February 3, 2025 21:04
@makubacki makubacki self-assigned this Feb 3, 2025
@github-actions github-actions bot added the type:documentation Improvements or additions to documentation label Feb 3, 2025
Adds a new `MM_STANDALONE` driver that is used to link against a
`PerformanceLib` core instance for MM user mode drivers.

An empty driver is reserved for this use case for the following
reasons:

1. Make integration and debug obvious and easy by having a clearly
   named and designated driver for this purpose.
2. To be among the first MM user mode drivers dispatched to install
   the performance protocol and make the performance service
   available to other user mode drivers as early as possible.
3. To avoid linking directly to `MmSupervisorRing3Broker`. The MM
   Core performance library instance code has functionality that
   depends on MM services to be available. Since the Ring 3 Broker
   is reponsible for initializing MM services in user mode, it is
   complicated and error prone to attempt to defer using those
   services to install the performance protocol until syscall has
   been set up with the core and a proper `gMmst` global could be
   used by the common MM code. This is all much simpler and easier
   to integrate in a separate MM user mode driver.
4. To avoid linking the library to a pre-existing MM module that
   may have an unknown number of depdencies in its DEPEX. Since
   the DEPEX on a given platform aggregates all of the dependencies
   in linked libraries, it is not safe to assume any given MM module
   not dedicated exclusively for this purpose will not have unrelated
   dependencies that unnecessarily prolong installation of the
   performance protocol.

Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Temporarily use this branch for testing.

This commit will be reverted after the following Mu Basecore PR is
merged:

microsoft/mu_basecore#1268

Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
@codecov-commenter
Copy link

codecov-commenter commented Feb 4, 2025

Codecov Report

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

Project coverage is 0.50%. Comparing base (f29899e) to head (763154a).

Files with missing lines Patch % Lines
...sorRing3Performance/MmSupervisorRing3Performance.c 0.00% 9 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main    #390      +/-   ##
========================================
- Coverage   0.50%   0.50%   -0.01%     
========================================
  Files        142     143       +1     
  Lines      20603   20612       +9     
  Branches      60      60              
========================================
  Hits         105     105              
- Misses     20493   20502       +9     
  Partials       5       5              
Flag Coverage Δ
MmSupervisorPkg 0.50% <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.

@@ -189,7 +189,7 @@ def GetDependencies(self):
{
"Path": "MU_BASECORE",
"Url": "https://github.com/microsoft/mu_basecore.git",
"Branch": "dev/202405"
"Branch": "feature/add_standalonemm_perf_lib"
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment is just a reminder to restore the basecore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:documentation Improvements or additions to documentation type:feature-request A new feature proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Add MM Performance Record Support
4 participants