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

SignedCapsulePkg: Allow hex FirmwareType values in ini files #6475

Conversation

bexcran
Copy link
Contributor

@bexcran bexcran commented Nov 27, 2024

Description

Some platforms such as Ampere Altra have special values for the FirmwareType field: for example 2147483649 is for SCP updates.

To make such values easier to read/understand, allow them to be written in hex: in this case it would be 0x80000001. Implement this by attempting to get a hex integer from the ini data file if reading a decimal value fails.

  • Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

Write a SystemFirmwareUpdateConfig.ini file with a FirmwareType value of 0x80000002 and verified it was correctly read and interpreted when applying a capsule update.

Integration Instructions

N/A

@bexcran bexcran force-pushed the allow-hex-firmwaretype-capsule-updates branch 2 times, most recently from 4c2fff1 to 0f9f382 Compare December 2, 2024 15:22
@bexcran
Copy link
Contributor Author

bexcran commented Dec 18, 2024

@ajfish @leiflindholm @mdkinney Could someone review this please?

Some platforms such as Ampere Altra have special values for the
FirmwareType field: for example 2147483649 is for SCP updates.

To make such values easier to read/understand, allow them to be
written in hex: in this case it would be 0x80000001. Implement this
by attempting to get a hex integer from the ini data file if reading
a decimal value fails.

Signed-off-by: Rebecca Cran <[email protected]>
@bexcran bexcran force-pushed the allow-hex-firmwaretype-capsule-updates branch from 0f9f382 to f2fd186 Compare December 18, 2024 00:58
Copy link
Member

@leiflindholm leiflindholm left a comment

Choose a reason for hiding this comment

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

Looks like an excellent idea to me.
Only question would be if there is any relevant documentation that should be updated to match.

@mdkinney
Copy link
Member

I would prefer to see platforms move to FmpDevicePkg and stop using SignedCapsulePkg so the SignedCapsulePkg can be deleted. That would be better than making enhancements. How many platforms need to be updated?

@leiflindholm
Copy link
Member

I would prefer to see platforms move to FmpDevicePkg and stop using SignedCapsulePkg so the SignedCapsulePkg can be deleted. That would be better than making enhancements. How many platforms need to be updated?

Oh, err, good point.

@bexcran
Copy link
Contributor Author

bexcran commented Dec 18, 2024

I would prefer to see platforms move to FmpDevicePkg and stop using SignedCapsulePkg so the SignedCapsulePkg can be deleted. That would be better than making enhancements. How many platforms need to be updated?

Thanks - I'm still figuring all this capsule stuff out.
Platforms using SignedCapsulePkg:

Platform/Hisilicon/D03
Platform/Hisilicon/D05
Platform/Hisilicon/D06
Silicon/Marvell/Armada7k8k
Platform/Intel/QuarkPlatformPkg
Silicon/Socionext/SynQuacer
Silicon/AMD/Styx
Platform/Intel/Vlv2TbltDevicePkg/
Platform/AMD/VanGoghBoard
Platform/AMD/OverdriveBoard
Platform/Ampere/JadePkg
Platform/Socionext/SynQuacerEvalBoard
Platform/Socionext/DeveloperBox

Given it's in both QuarkPlatform and Vlv2 (i.e. MinnowBoard) I suspect that's where people get examples of how to do capsule updates from.

Is there a good place we should be getting examples - i.e. using FmpDevicePkg?

@leiflindholm
Copy link
Member

I think the following Arm-based platforms are probably ready to be archived:

  • Platform/Hisilicon/D03
  • Platform/Hisilicon/D05
  • Platform/Hisilicon/D06
  • Silicon/Socionext/SynQuacer
  • Silicon/AMD/Styx @ardbiesheuvel @changab ?
  • Platform/Socionext/SynQuacerEvalBoard
  • Platform/Socionext/DeveloperBox

and possibly

@ardbiesheuvel
Copy link
Member

archived being a euphemism for deletion?

@leiflindholm
Copy link
Member

archived being a euphemism for deletion?

Well, moved off the main branch to an archive branch frozen at that point in time.

@ardbiesheuvel
Copy link
Member

archived being a euphemism for deletion?

Well, moved off the main branch to an archive branch frozen at that point in time.

Fair enough. Fine with me for all platforms listed.

@bexcran
Copy link
Contributor Author

bexcran commented Dec 18, 2024

Those all sound good to me too. I no longer have a DeveloperBox or MACCHIATObin (Armada 8040) or even a Honeycomb (LX2160) so I won't complain. :)

@mdkinney
Copy link
Member

How about a tag and delete from main branch? Can always retrieve from the tag.

@bexcran
Copy link
Contributor Author

bexcran commented Dec 18, 2024

How about a tag and delete from main branch? Can always retrieve from the tag.

I think that makes a lot of sense, especially since the date of the tag will indicate which revision of edk2 to use with it.

Copy link

This PR has been automatically marked as stale because it has not had activity in 60 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

@github-actions github-actions bot added the stale Due to lack of updates, this item is pending deletion. label Feb 16, 2025
Copy link

This pull request has been automatically been closed because it did not have any activity in 60 days and no follow up within 7 days after being marked stale. Thank you for your contributions.

@github-actions github-actions bot closed this Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Due to lack of updates, this item is pending deletion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants