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.

Sign up for GitHub

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

Jump to bottom

[Bug]: Cannot use ConfigGet*** APIs from ConfigClientGenerated.h in Pre-Memory envrionment #243

Closed
1 task done
MarcChen46 opened this issue Sep 19, 2023 · 1 comment · Fixed by #244
Closed
1 task done

[Bug]: Cannot use ConfigGet*** APIs from ConfigClientGenerated.h in Pre-Memory envrionment #243

MarcChen46 opened this issue Sep 19, 2023 · 1 comment · Fixed by #244
Labels
state:needs-maintainer-feedback Needs more information from a maintainer to determine next steps state:needs-owner Needs an issue owner to be assigned state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working urgency:high Significant with a critical impact

Comments

@MarcChen46
Copy link
Contributor

Is there an existing issue for this?

  • I have searched existing issues

Current Behavior

If the ConfigPolicyTo[Silicon|Platform]Mapper driver get dispatched in Pre-Memory phase, when we try to use ConfigGet*** APIs from ConfigClientGenerated.h to retrieve the config back, all the data is 0 due to the memory is still not ready to use, and in each ConfigGet*** APIs, it calls InitConfigPolicyCache() to get CloudConfigPolicy and store in CachedPolicy, which is a global variable in C code, so in Pre-Memory phase, the global variable cannot be updated success, all the data will be kept as 0.

Expected Behavior

To make the design generic and can be adopted to all system architecture, needs to consider the case that ConfigPolicyTo[Silicon|Platform]Mapper get dispatched in Pre-Memory phase, and ConfigGet*** APIs from ConfigClientGenerated.h should work properly in Pre-Memory environment, so global variables consumption in Pre-Memory phase should be prevented.

Steps To Reproduce

Call ConfigGet*** APIs from ConfigClientGenerated.h, the return status is success, but all the data we retrieved will still be zero.

Build Environment

- OS(s): Windows 11
- Tool Chain(s): VS2022
- Targets Impacted: RELEASE, DEBUG

Version Information

v1.0.0

Urgency

High

Are you going to fix this?

Someone else needs to fix it

Do you need maintainer feedback?

Maintainer feedback requested

Anything else?

No response
@MarcChen46 MarcChen46 added state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working labels Sep 19, 2023
@github-actions github-actions bot added state:needs-maintainer-feedback Needs more information from a maintainer to determine next steps state:needs-owner Needs an issue owner to be assigned urgency:high Significant with a critical impact labels Sep 19, 2023
@MarcChen46
Copy link
Contributor Author

@os-d @kuqin12 @apop5

@MarcChen46 MarcChen46 changed the title [Bug]: Ca [Bug]: Cannot use ConfigGet*** APIs from ConfigClientGenerated.h in Pre-Memory envrionment Sep 19, 2023
@kuqin12 kuqin12 mentioned this issue Sep 19, 2023
Merged
5 tasks
kuqin12 added a commit that referenced this issue Sep 20, 2023
@kuqin12
Updated auto-gen script to accept cache for policy values (#244)
4da0c03 
# Preface

Please ensure you have read the [contribution
docs](https://github.com/microsoft/mu/blob/master/CONTRIBUTING.md) prior
to submitting the pull request. In particular,
[pull request
guidelines](https://github.com/microsoft/mu/blob/master/CONTRIBUTING.md#pull-request-best-practices).

## Description

This change adds a set of getter functions that accepts a caller
designated buffer for cached configuration policy. This will avoid the
direct usage of updating global variable does not work (i.e. the module
is XIP).

Fixes #243.

For each item, place an "x" in between `[` and `]` if true. Example:
`[x]`.
_(you can also check items in the GitHub UI)_

- [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, ...
- [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, ...

## How This Was Tested

The auto-generated headers were built on QEMU Q35 and confirmed the GOP
configuration knob works as expected.

## Integration Instructions

N/A for existing platforms. For platforms that require XIP usage, one
should use `ConfigGet***FromCache` with user designated buffer, where
the size of the buffer is specified in `CACHED_POLICY_SIZE`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
state:needs-maintainer-feedback Needs more information from a maintainer to determine next steps state:needs-owner Needs an issue owner to be assigned state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working urgency:high Significant with a critical impact
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Updated auto-gen script to accept cache for policy values kuqin12/mu_feature_config
1 participant
@MarcChen46