Attention is currently required from: Edward O'Callaghan, Felix Singer, Nico Huber, Angel Pons, Anastasia Klimchuk. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60055 )
Change subject: SFDP: drop static table length check to make newer SFDP revisions work ......................................................................
Patch Set 2:
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/60055/comment/962afb88_c137d855 PS1, Line 28: ***RFC*** We could maintain a list of revisions/lengths but is it really : worth it? :
IMHO, if the SFDP specs *mandate* a specific length, we could check for it. However, if that is not the case, I think it won't be worth it in the long run.
It does indirectly by specifying the mandatory table entries count. However, for the optional ones the length apparently is table type specific.
Revisions, table lengths: ``` pre-JESD216 (Intel): 4 DWORDs (16b) JESD216 (r1.0): 9 DWORDs (36b) JESD216A (r1.5 [sic!]): 16 DWORDS (64b) JESD216B (r1.6): same len but definition of prev. rsv bits JESD216C (r1.7**): 20 DWORDs (80b) JESD216D (r1.8**): no table changes JESD216D.01 (r1.8**): no table changes JESD216E (r1.9**): no table changes JESD216F (r1.A): 23 DWORDs (92b) ```
** I couldn't find JESD216C,D,D.01,E anywhere, so r1.7-r1.9 are just educated guesses based on some mentions in the document history
We still need to check the revision number to properly parse the data (i.e. only parse newer fields if the revision is high enough).
Currently, we don't really need since we don't need any of the new fields. That may change, though. We could do the check where some higher field is being read, then.
Commit Message:
https://review.coreboot.org/c/flashrom/+/60055/comment/f6f64602_5ccf18f3 PS2, Line 9: JESD216F (1.6) released in September 2021 : adds three new DWORDs JESD216A r1.5: adds 5 new DWORDs. ... later revisions add even more... so any chip with SFDP >= r1.5 is broken currently
https://review.coreboot.org/c/flashrom/+/60055/comment/64a31d91_95235cd3 PS2, Line 11: 32 9*4=36.
https://review.coreboot.org/c/flashrom/+/60055/comment/49387239_881d16a6 PS2, Line 24: let's add the output here
``` Probing for Unknown SFDP-capable chip, 0 kB: SFDP revision = 1.0 SFDP number of parameter headers is 2 (NPH = 1).
SFDP parameter table header 0/1: ID 0x00, version 1.0 Length 36 B, Parameter Table Pointer 0x000030
SFDP parameter table header 1/1: ID 0xc8, version 1.0 Length 12 B, Parameter Table Pointer 0x000060 ```
``` Probing for Unknown SFDP-capable chip, 0 kB: SFDP revision = 1.6 SFDP number of parameter headers is 2 (NPH = 1).
SFDP parameter table header 0/1: ID 0x00, version 1.6 Length 64 B, Parameter Table Pointer 0x000030
SFDP parameter table header 1/1: ID 0xc8, version 1.0 Length 12 B, Parameter Table Pointer 0x000090 ```