linuxboot
pkg/amd/firmware: Wrap PSP and BIOS directories in arrays
This allows for returning multiple directories. Up until now, only the first directories would be returned, while many firmware images contain multiple directories for various SoC families and models, e.g., for desktop boards where SoCs can be swapped.
Signed-off-by: Daniel Maslowski [email protected]
Codecov Report
Merging #367 (f581bcd) into master (64c18ae) will decrease coverage by
0.02%. The diff coverage is0.00%.
@@ Coverage Diff @@
## master #367 +/- ##
==========================================
- Coverage 41.74% 41.72% -0.03%
==========================================
Files 110 110
Lines 7649 7653 +4
==========================================
Hits 3193 3193
- Misses 3877 3881 +4
Partials 579 579
| Impacted Files | Coverage Δ | |
|---|---|---|
| pkg/amd/manifest/firmware.go | 0.00% <0.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 64c18ae...f581bcd. Read the comment docs.
A slice doesn't provide information about the origin of the element. It is unclear what element should I use, if I end up having multiple.
A slice doesn't provide information about the origin of the element. It is unclear what element should I use, if I end up having multiple.
Yea, that is also currently the problem: I don't know what I got, I only got a single directory, and I am missing the other directories. From the code I could understand that I got the first directory that is found. And it could be either from the EmbeddedFirmwareStruct or it could have been found by scanning. When the latter is the case, how would we know the processor family/model range?
To expost that information when we have it, I would suggest adding metadata to the respective directory, such as processor family / model range based on the offset. However, that is a different concern; this PR here is to enable multiple directories for a start. WDYT?
A slice doesn't provide information about the origin of the element. It is unclear what element should I use, if I end up having multiple.
Yea, that is also currently the problem: I don't know what I got, I only got a single directory, and I am missing the other directories. From the code I could understand that I got the first directory that is found. And it could be either from the EmbeddedFirmwareStruct or it could have been found by scanning. When the latter is the case, how would we know the processor family/model range?
To expost that information when we have it, I would suggest adding metadata to the respective directory, such as processor family / model range based on the offset. However, that is a different concern; this PR here is to enable multiple directories for a start. WDYT?
Sorry, for a very long delay. There is a documentation problem, each "directory" has its own purpose. The future code should clearly specify the purpose of each directory. (which we currently can't do)
I agree that the current code assumes, that there could be no more than a single directory in firmware, which could be incorrect.
I propose to leave everything as is, but move "scanning" into a separate function
Okay I will create another implementation then. We discussed this in the Fiano sync call, and the proposal is to rework the directory structs, so that they
- contain the range; it's currently side by side, though belongs to the dir
- get a source field; can be "scan", "efs". "xxxdir", depending on whether it resulted from seeking or from following references via EFS or possibly a level 1 directory (called pointers here in the code; "xxxdir" would need be more precise of course such as "fam-17h_model-00h-0fh_biosdir" or something)