Skip to content

Support Turin. #134

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
Merged
merged 1 commit into from
Jul 11, 2024
Merged

Support Turin. #134

merged 1 commit into from
Jul 11, 2024

Conversation

daym
Copy link
Collaborator

@daym daym commented May 27, 2024

Fixes #122.

@daym daym requested a review from dancrossnyc May 27, 2024 18:52
@daym daym force-pushed the issue-122 branch 5 times, most recently from 4222470 to 65bb6d1 Compare May 27, 2024 19:04
dancrossnyc
dancrossnyc reviewed May 28, 2024
View reviewed changes
src/entry.rs
let v = s.iter().collect::<Vec<_>>();
state.serialize_field("MemDfeSearchElement20", &v)?;
} else if let Some(s) = self.body_as_struct_array::<memory::MemDfeSearchElement32>() {
// } else if let Some(s) = self.body_as_struct_array::<memory::MemDfeSearchElement32>() { // UH OH
Copy link
Contributor

Choose a reason for hiding this comment

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

UH OH?

Copy link
Collaborator Author

@daym daym May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah. There seem to be two different versions of struct MemDfeSearchElement in use (the number is the size in bytes in decimal).

Since those structs are used in a struct array and there's only a total_size of the entire array somewhere it could (and did) happen that it's misdetected since it's possible that sizeof(MemDfeSearchElement32) divides total_size and also sizeof(MemDfeSearchElement36) divides total_size at the same time (for example total_size = 288 or 576 or 864 or ...). Which struct definition should then be used?

This means that the dump functionality could pick up the wrong struct definition.
It also makes me wonder how Abl knows which one it is when booting.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Analysis of what we have: 19 of 54 directories in fluffy-tribble do not have that struct size = 36.

Details: Only the following fail assert sizeof == 36:

./am5-1.0.0.6/AgesaPkg/Addendum/Apcb/RaphaelAm5/ApcbDataDefaultRecovery/CRB_Splinter
./am5-1.0.0.6/AgesaPkg/Addendum/Apcb/RaphaelAm5Alt1/ApcbDataDefaultRecovery/CRB_Shredder
./genoa-1.0.0.3/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/GenoaCommon
./genoa-1.0.0.3/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby
./genoa-1.0.0.9/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/GenoaCommon
./genoa-1.0.0.9/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Cinnabar
./genoa-1.0.0.9/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby
./genoa-1.0.0.9/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Sunstone
./genoa-1.0.0.a/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/GenoaCommon
./genoa-1.0.0.a/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Cinnabar
./genoa-1.0.0.a/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby
./genoa-1.0.0.a/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby
./genoa-1.0.0.a/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Sunstone
./genoa-1.0.0.b/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/GenoaCommon
./genoa-1.0.0.b/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Cinnabar
./genoa-1.0.0.b/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby
./genoa-1.0.0.b/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Sunstone
./genoa-1.0.0.8/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/GenoaCommon
./genoa-1.0.0.8/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby

Copy link
Collaborator Author

@daym daym May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Used struct sizes of that struct are (... they've got to be kidding me):

Size/B HeaderSize/B PayloadSize/B name struct_dir PayloadExtSize/B
36 12 12 ./turin-0.0.7.3/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Purico 12
36 12 12 ./turin-0.0.7.3/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/BreithornCommon 12
36 12 12 ./turin-0.0.7.3/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12
36 12 12 ./turin-0.0.7.1/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Purico 12
36 12 12 ./turin-0.0.7.1/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Onyx 12
36 12 12 ./turin-0.0.7.1/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/BreithornCommon 12
36 12 12 ./turin-0.0.7.1/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Galena 12
36 12 12 ./turin-0.0.7.1/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12
36 12 12 ./turin-0.0.7.0/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Purico 12
36 12 12 ./turin-0.0.7.0/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Onyx 12
36 12 12 ./turin-0.0.7.0/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/BreithornCommon 12
36 12 12 ./turin-0.0.7.0/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Galena 12
36 12 12 ./turin-0.0.7.0/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12
36 12 12 ./turin-0.0.8.0/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Purico 12
36 12 12 ./turin-0.0.8.0/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/BreithornCommon 12
36 12 12 ./turin-0.0.8.0/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12
24 12 12 ./am5-1.0.0.6/AgesaPkg/Addendum/Apcb/RaphaelAm5/ApcbDataDefaultRecovery/CRB_Splinter 0
24 12 12 ./am5-1.0.0.6/AgesaPkg/Addendum/Apcb/RaphaelAm5Alt1/ApcbDataDefaultRecovery/CRB_Shredder 0
36 12 12 ./turin-0.0.8.1/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Purico 12
36 12 12 ./turin-0.0.8.1/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/BreithornCommon 12
36 12 12 ./turin-0.0.8.1/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12
36 12 12 ./turin-0.0.7.2/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Purico 12
36 12 12 ./turin-0.0.7.2/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/BreithornCommon 12
36 12 12 ./turin-0.0.7.2/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12
36 12 12 ./am5-1.1.7.0/AGESA/AgesaPkg/Addendum/Apcb/RaphaelAm5/ApcbDataDefaultRecovery/CRB_Splinter 12
36 12 12 ./am5-1.1.7.0/AGESA/AgesaPkg/Addendum/Apcb/GraniteRidgeAm5Alt1/ApcbDataDefaultRecovery/CRB_Shredder 12
36 12 12 ./am5-1.1.7.0/AGESA/AgesaPkg/Addendum/Apcb/GraniteRidgeAm5/ApcbDataDefaultRecovery/CRB_Splinter 12
36 12 12 ./am5-1.1.7.0/AGESA/AgesaPkg/Addendum/Apcb/RaphaelAm5Alt1/ApcbDataDefaultRecovery/CRB_Shredder 12
36 12 12 ./fire-range-0.0.6.0/AGESA/AgesaPkg/Addendum/Apcb/GraniteRidgeFl1/ApcbDataDefaultRecovery/CRB_Vortex 12
20 8 12 ./genoa-1.0.0.3/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/GenoaCommon 0
20 8 12 ./genoa-1.0.0.3/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby 0
32 8 12 ./genoa-1.0.0.9/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/GenoaCommon 12
32 8 12 ./genoa-1.0.0.9/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Cinnabar 12
32 8 12 ./genoa-1.0.0.9/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12
32 8 12 ./genoa-1.0.0.9/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Sunstone 12
36 12 12 ./turin-0.0.7.4/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Purico 12
36 12 12 ./turin-0.0.7.4/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/BreithornCommon 12
36 12 12 ./turin-0.0.7.4/AgesaPkg/Addendum/Apcb/TurinSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12
36 12 12 ./turin-0.0.7.4/AgesaPkg/Addendum/Apcb/VeniceSp7Rdimm/ApcbDataDefaultRecovery/Purico 12
36 12 12 ./turin-0.0.7.4/AgesaPkg/Addendum/Apcb/VeniceSp7Rdimm/ApcbDataDefaultRecovery/WeisshornCommon 12
36 12 12 ./turin-0.0.7.4/AgesaPkg/Addendum/Apcb/VeniceSp7Rdimm/ApcbDataDefaultRecovery/Ruby 12
32 8 12 ./genoa-1.0.0.a/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/GenoaCommon 12
32 8 12 ./genoa-1.0.0.a/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Cinnabar 12
32 8 12 ./genoa-1.0.0.a/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12
32 8 12 ./genoa-1.0.0.a/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12
32 8 12 ./genoa-1.0.0.a/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Sunstone 12
32 8 12 ./genoa-1.0.0.b/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/GenoaCommon 12
32 8 12 ./genoa-1.0.0.b/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Cinnabar 12
32 8 12 ./genoa-1.0.0.b/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12
32 8 12 ./genoa-1.0.0.b/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Sunstone 12
36 12 12 ./am5-1.1.0.2b/AGESA/AgesaPkg/Addendum/Apcb/RaphaelAm5/ApcbDataDefaultRecovery/CRB_Splinter 12
36 12 12 ./am5-1.1.0.2b/AGESA/AgesaPkg/Addendum/Apcb/RaphaelAm5Alt1/ApcbDataDefaultRecovery/CRB_Shredder 12
32 8 12 ./genoa-1.0.0.8/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/GenoaCommon 12
32 8 12 ./genoa-1.0.0.8/AgesaPkg/Addendum/Apcb/GenoaSp5Rdimm/ApcbDataDefaultRecovery/Ruby 12

dancrossnyc
dancrossnyc reviewed May 28, 2024
View reviewed changes
Copy link
Contributor

@dancrossnyc dancrossnyc left a comment

Choose a reason for hiding this comment

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

There are some lints that need to be fixed?

@daym daym force-pushed the issue-122 branch 2 times, most recently from 8c66138 to a3a367b Compare June 28, 2024 00:06
@daym daym marked this pull request as ready for review July 11, 2024 18:10
@dancrossnyc dancrossnyc merged commit 36dc176 into main Jul 11, 2024
4 checks passed
@dancrossnyc dancrossnyc deleted the issue-122 branch July 11, 2024 18:46
@daym daym mentioned this pull request Aug 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dancrossnyc dancrossnyc dancrossnyc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Turin
2 participants
@daym @dancrossnyc