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

Make PlatformInitPreMem independent to IntelSiliconPkg and IntelFsp2WrapperPkg #293

Conversation

PaddyDengKC
Copy link
Contributor

@PaddyDengKC PaddyDengKC commented Sep 5, 2024

Description

This driver is shared across different Silicon vendors. So it should not use any Intel specific packages.

In order to achieve this, added a new lib FspSupportLib to isolate silicon related PCDs

  • 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

CI build passed.

Integration Instructions

No additional change is required for the consuming platforms.

@github-actions github-actions bot added the impact:non-functional Does not have a functional impact label Sep 5, 2024
@PaddyDengKC
Copy link
Contributor Author

@microsoft-github-policy-service agree company="Microsoft"

@PaddyDengKC
Copy link
Contributor Author

Hi @makubacki @Javagedes @os-d @apop5,

Could you please help to review this PR?

Regards,
Paddy

@PaddyDengKC PaddyDengKC force-pushed the user/v-paddydeng/IndepPlatformInitPreMem branch from 0f1c877 to 2302a3e Compare October 24, 2024 07:01
PlatformInitPreMemNonFsp provides identical function compares to PlatformInitPreMem.
But using FspSupportNull.c thus does not require IntelSiliconPkg and IntelFsp2WrapperPkg as dependency.
@PaddyDengKC PaddyDengKC force-pushed the user/v-paddydeng/IndepPlatformInitPreMem branch from 2302a3e to fdc6284 Compare October 24, 2024 07:04
@PaddyDengKC PaddyDengKC force-pushed the user/v-paddydeng/IndepPlatformInitPreMem branch from 3bab8f4 to 99ab205 Compare October 28, 2024 02:36
@PaddyDengKC
Copy link
Contributor Author

Hi @makubacki ,

I modified the PR based on tianocore/edk2-platforms#224
Please help to review the current code change. Thanks.

Regards,
Paddy

@makubacki
Copy link
Member

@PaddyDengKC, there are a few things that the upstream maintainers missed that should be fixed. Please make an upstream change with those fixes and then amend this PR with them.

  1. Add file headers with copyright to newly added files where it is missing.
  2. Add an empty line to the end of files where it is missing. Some tools may depend on that.
  3. Add the new library instance to the DSC file so it is not a Mu override.

@PaddyDengKC
Copy link
Contributor Author

Hi @makubacki ,

I pushed a commit to update as you suggested.
The same change has been merged to edk2-platform too: tianocore/edk2-platforms#236.
Please help to review it. Thanks.

Regards,
Paddy

@makubacki
Copy link
Member

I pushed a commit to update as you suggested. The same change has been merged to edk2-platform too

Thanks Paddy

@makubacki makubacki requested review from kenlautner and apop5 October 31, 2024 16:18
@makubacki
Copy link
Member

Pipelines failed due to a temporary issue with a submodule missing that has been resolved in the latest Mu Basecore. Restarted the pipeline runes.

@makubacki makubacki merged commit 175663e into microsoft:release/202405 Nov 5, 2024
6 of 8 checks passed
makubacki added a commit to makubacki/mu_common_intel_min_platform that referenced this pull request Nov 5, 2024
…rapperPkg (microsoft#293)

## Description

This driver is shared across different Silicon vendors. So it should not
use any Intel specific packages.

In order to achieve this, added a new lib `FspSupportLib` to isolate
silicon related PCDs

- [ ] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

CI build passed.

## Integration Instructions

No additional change is required for the consuming platforms.

---------

Co-authored-by: Michael Kubacki <[email protected]>
makubacki added a commit that referenced this pull request Nov 5, 2024
…rapperPkg (#293)

## Description

This driver is shared across different Silicon vendors. So it should not
use any Intel specific packages.

In order to achieve this, added a new lib `FspSupportLib` to isolate
silicon related PCDs

- [ ] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

CI build passed.

## Integration Instructions

No additional change is required for the consuming platforms.

---------

Co-authored-by: Michael Kubacki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:non-functional Does not have a functional impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants