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

[Bug]: DXE Sets EFI_MEMORY_SP to GCDReserved #884

Closed
1 task done
os-d opened this issue Jun 3, 2024 · 0 comments · Fixed by #886
Closed
1 task done

[Bug]: DXE Sets EFI_MEMORY_SP to GCDReserved #884

os-d opened this issue Jun 3, 2024 · 0 comments · Fixed by #886
Assignees
Labels
state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working urgency:medium Important with a moderate impact

Comments

@os-d
Copy link
Contributor

os-d commented Jun 3, 2024

Is there an existing issue for this?

  • I have searched existing issues

Current Behavior

Currently, when a resource descriptor HOB with EFI_MEMORY_SP indicated is receiving by DxeCore, it will create a GcdReserved region for it, regardless of the HOB memory type.

Expected Behavior

DxeCore should respect the memory type of the HOB.

Steps To Reproduce

See code.

Build Environment

- OS(s):
- Tool Chain(s):
- Targets Impacted:
All

Version Information

top of tree

Urgency

Medium

Are you going to fix this?

I will fix it

Do you need maintainer feedback?

No maintainer feedback needed

Anything else?

No response

@os-d os-d added state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working labels Jun 3, 2024
@github-actions github-actions bot added the urgency:medium Important with a moderate impact label Jun 3, 2024
@os-d os-d closed this as completed in #886 Jun 5, 2024
os-d added a commit that referenced this issue Jun 5, 2024
## Description

When supplying DxeCore with a resource descriptor HOB, a platform can
choose which memory type to specify. For EFI_MEMORY_SP resource
descriptor HOBs, instead of blindly setting GcdReserved as the memory
type, respect what the resource descriptor HOB specified. Closes #884.

- [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 virtual platforms with CXL memory attached.

## Integration Instructions

N/A.
makubacki pushed a commit to makubacki/mu_basecore that referenced this issue Jun 18, 2024
## Description

When supplying DxeCore with a resource descriptor HOB, a platform can
choose which memory type to specify. For EFI_MEMORY_SP resource
descriptor HOBs, instead of blindly setting GcdReserved as the memory
type, respect what the resource descriptor HOB specified. Closes microsoft#884.

- [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 virtual platforms with CXL memory attached.

## Integration Instructions

N/A.
makubacki pushed a commit that referenced this issue Jun 18, 2024
## Description

When supplying DxeCore with a resource descriptor HOB, a platform can
choose which memory type to specify. For EFI_MEMORY_SP resource
descriptor HOBs, instead of blindly setting GcdReserved as the memory
type, respect what the resource descriptor HOB specified. Closes #884.

- [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 virtual platforms with CXL memory attached.

## Integration Instructions

N/A.
os-d added a commit to os-d/mu_basecore that referenced this issue Jul 25, 2024
## Description

When supplying DxeCore with a resource descriptor HOB, a platform can
choose which memory type to specify. For EFI_MEMORY_SP resource
descriptor HOBs, instead of blindly setting GcdReserved as the memory
type, respect what the resource descriptor HOB specified. Closes microsoft#884.

- [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 virtual platforms with CXL memory attached.

## Integration Instructions

N/A.
os-d added a commit to os-d/mu_basecore that referenced this issue Aug 23, 2024
## Description

When supplying DxeCore with a resource descriptor HOB, a platform can
choose which memory type to specify. For EFI_MEMORY_SP resource
descriptor HOBs, instead of blindly setting GcdReserved as the memory
type, respect what the resource descriptor HOB specified. Closes microsoft#884.

- [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 virtual platforms with CXL memory attached.

## Integration Instructions

N/A.
os-d added a commit that referenced this issue Aug 24, 2024
## Description

When supplying DxeCore with a resource descriptor HOB, a platform can
choose which memory type to specify. For EFI_MEMORY_SP resource
descriptor HOBs, instead of blindly setting GcdReserved as the memory
type, respect what the resource descriptor HOB specified. Closes #884.

- [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 virtual platforms with CXL memory attached.

## Integration Instructions

N/A.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working urgency:medium Important with a moderate impact
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant