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

add alternative header ending signature #141

Merged
merged 1 commit into from
Nov 7, 2024
Merged

add alternative header ending signature #141

merged 1 commit into from
Nov 7, 2024

Conversation

orangecms
Copy link
Contributor

I don't know if this is a typo here in amd-apcb or both are possible.

I have seen "BCBA" in a firmware image from an ASRock A520M-HVS board, version A52MIX_2.73.
And it is also in Fiano (just in different notation):
https://github.com/linuxboot/fiano/blob/d844cd98141518f41b3e4012bcf419984074a2b4/pkg/amd/apcb/internal.go#L16

@daym
Copy link
Collaborator

daym commented Nov 4, 2024

Hi!

Hmm, not sure what's up with that.

AMD docs for Turin, Genoa, Milan, Rome definitely say "BCPA" (which is "APCB" backward, ha ha).

But it might be that actual images always say "BCBA" ?

Could you maybe check a few images so we get an overview of what's the "compatible" thing to do? Is it ever "BCPA" at all?

@daym daym self-assigned this Nov 4, 2024
@daym
Copy link
Collaborator

daym commented Nov 4, 2024

I've checked all the bioses we got for the AMD evaluation boards and they all say BCPA.

@orangecms
Copy link
Contributor Author

orangecms commented Nov 4, 2024 •
edited
Loading

Alright, soooo... it looks like this occurs here and there. From a few samples, it looks like it correlates with the OEM.
Maybe it's intentional, maybe someone made a typo somewhere and it just happened to be this way now.

The following boards have BCPA:

  • Gigabyte X870E AORUS ELITE WIFI7 (fw ver f2)
  • Gigabyte X870E AORUS PRO ICE (fw ver f1)
  • ASUS GX650PY (fw ver 3.23)
  • ASUS ROG STRIX B550-I

Whereas those have BCBA:

  • ASRock X570 Phantom Gaming 4 (fw ver 5.63)
  • ASRock X370 Taichi (fw ver 7.10)
  • Google Skyrim (blob is in coreboot 3rdparty/blobs/mainboard/google/skyrim/APCB_MDN_D5.bin)
  • Google Guybrush (blob in coreboot 3rdparty/blobs/mainboard/google/guybrush/APCB_CZN_D4.bin)
  • AMD Majolica (blob in coreboot 3rdparty/blobs/mainboard/amd/majolica/APCB_CZN_D4.bin)

BCPA just being APCB backwards makes sense to me. And the other one could be happenstance. 🤷

@daym
Copy link
Collaborator

daym commented Nov 5, 2024 •
edited
Loading

Thanks for checking!

Well, I guess we can merge the PR.

Just wanted to make sure this situation doesn't hide something way worse first.

Could you add a short comment to the source code that the case is undocumented but happens in the wild? Just so we don't scratch our heads later.

Also, please create a github issue (for example using the "..." thing in the PR and "Reference in new issue") and refer to it in the commit message like this:

Fixes <https://github.com/oxidecomputer/amd-apcb/issues/xxx>.

@orangecms
Copy link
Contributor Author

Will do, thank you!

@orangecms
Copy link
Contributor Author

resolves #142

@orangecms
add alternative header ending signature
45d382d 
This is found in ASRock firmware images, e.g. A520M-HVS A52MIX_2.73.
Also seen in coreboot 3rd_party/blobs/mainboard/, and used in Fiano:
https://github.com/linuxboot/fiano/blob/d844cd98141518f41b3e4012bcf419984074a2b4/pkg/amd/apcb/internal.go#L16

Fixes <oxidecomputer#142>.

Signed-off-by: Daniel Maslowski <info@orangecms.org>
@daym daym merged commit 034aea5 into oxidecomputer:main Nov 7, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daym daym daym approved these changes

Assignees

@daym daym

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@orangecms @daym