-
Notifications
You must be signed in to change notification settings - Fork 39
Add AdvLoggerLocator and Variable Policy Interface #608
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
base: main
Are you sure you want to change the base?
Conversation
beffa91 to
1c56a74
Compare
03db923 to
ea6c090
Compare
a265dfb to
25a740b
Compare
25a740b to
29ac6ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather avoid too much Mu-specific content leaking throughout patina repo, especially branded as Mu (and in the SDK). Advanced logger is a Mu invention. As a component, it gets isolated within its component boundaries. This is replicating interfaces though that are controlled by Project Mu and expected to match the Mu author's definition. Therefore, I'd prefer it to go somewhere like mu_rust_helpers. Then, only the patina_adv_logger needs to take a dependency on that (it already does) and it's clear that this component depends on something from Project Mu.
(I know patina_sdk already has a mu_rust_helpers dependency. I'd like to minimize and eliminate that in the future).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'd requiring adding patina_sdk as a dependency of mu_rust_helpers which is a dependency of patina_sdk. Should I be worried about the circular dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the two dependencies EfiError and ProtocolInterface? If so, are those things you can just hookup in the component after using the main content from the Mu repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@makubacki Would you have an objection to adding only the aspects of variable policy found in Tianocore EDK2's VariablePolicyLib to patina_sdk/as a component service?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@makubacki The associated issue is low priority, but would you mind taking a look at this PR again?
…ablePolicy, and TargetVarState
4223421 to
4f83cae
Compare
4f83cae to
3b31d93
Compare
39c9e81 to
34c17ff
Compare
|
@kouchekiniad to create an issue with changes needed to help facilitate this change. |
Description
This change adds the publishing and policy-setting of an AdvLoggerLocator variable to match the Tianocore-based implementation of the core component of the Advanced Logger found in Mu's AdvLoggerPkg. To facilitate setting the policies of the AdvLoggerLocatorVariable, this change also implements a partial interface for the Mu Variable Policy Protocol.
How This Was Tested
Booted to Windows on physical platform, confirming that the AdvLoggerLocator variable has been successfully published and write-protected in Windows.
Integration Instructions
N/A