Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/63281 )
Change subject: util/spd_tools: Add ability to override SPD file for parts ......................................................................
util/spd_tools: Add ability to override SPD file for parts
This commit adds the ability to override the SPD file that is used for a specific part.
BUG=b:224884904 TEST=Verified that generated makefile uses specified SPD file and that it remains unchanged when this capability is not used
Signed-off-by: Robert Zieba robertzieba@google.com Change-Id: I078dd04fead2bf19f53bc6ca8295187d439adc20 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63281 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Rob Barnes robbarnes@google.com --- M util/spd_tools/README.md M util/spd_tools/src/part_id_gen/part_id_gen.go 2 files changed, 64 insertions(+), 27 deletions(-)
Approvals: build bot (Jenkins): Verified Rob Barnes: Looks good to me, approved
diff --git a/util/spd_tools/README.md b/util/spd_tools/README.md index 01bc417..6f09f56 100644 --- a/util/spd_tools/README.md +++ b/util/spd_tools/README.md @@ -459,14 +459,16 @@ * The memory technology used by the board, e.g. lp4x. * The path to the directory where the generated Makefile.inc should be placed. * A CSV file containing a list of the memory parts used by the board, with an -* optional fixed or exclusive ID for each part. A fixed ID is simply an integer -* and it ensure that part (and any that share the same SPD) will be assigned -* that ID. An exclusive ID is prefixed with `*` and ensures that only parts with -* the same exclusive ID will be assigned that ID, even if they would otherwise -* share the same ID. +* optional fixed or exclusive ID for each part and an optional SPD override file. +* A fixed ID is simply an integer and it ensure that part (and any that share the same SPD) +* will be assigned that ID. An exclusive ID is prefixed with `*` and ensures that +* only parts with the same exclusive ID will be assigned that ID, even if they would +* otherwise share the same ID. When using an SPD override file, the file will be searched +* for in the directory where mem_parts_used is located, if it is not found there then it +* will be searched for in the appropriate default spd directory. * NOTE: Only assign a fixed/exclusive ID if required for legacy reasons.
-Example of a CSV file using fixed and exclusive IDs: +Example of a CSV file using fixed and exclusive IDs, and SPD file overrides:
``` K4AAG165WA-BCWE,1 @@ -475,6 +477,8 @@ K4A8G165WC-BCWE H5AN8G6NDJR-XNC,8 H5ANAG6NCMR-XNC,*9 +H9HCNNNCPMMLXR-NEE,,H9HCNNNCPMMLXR-NEE.hex +H54G56CYRBX247,4,H54G56CYRBX247.hex ```
Explanation: This will ensure that the SPDs for K4AAG165WA-BCWE and diff --git a/util/spd_tools/src/part_id_gen/part_id_gen.go b/util/spd_tools/src/part_id_gen/part_id_gen.go index 65e07f4..edbad9c 100644 --- a/util/spd_tools/src/part_id_gen/part_id_gen.go +++ b/util/spd_tools/src/part_id_gen/part_id_gen.go @@ -102,9 +102,10 @@ )
type usedPart struct { - partName string - index int - mapping mappingType + partName string + index int + mapping mappingType + SPDOverride string }
func readPlatformsManifest(memTech string) (map[string]string, error) { @@ -183,30 +184,44 @@ }
if len(fields) == 1 { - parts = append(parts, usedPart{fields[0], -1, Auto}) - } else if len(fields) == 2 { + parts = append(parts, usedPart{fields[0], -1, Auto, ""}) + } else { var mapping = Auto var assignedId = -1 var err error = nil + var spdOverride string = ""
- if len(fields[1]) >= 2 && fields[1][0] == '*' { - // Exclusive mapping - mapping = Exclusive - assignedId, err = strconv.Atoi(fields[1][1:]) - } else { - mapping = Fixed - assignedId, err = strconv.Atoi(fields[1]) + // Second column, ID override + if len(fields) >= 2 { + if len(fields[1]) >= 2 && fields[1][0] == '*' { + // Exclusive mapping + mapping = Exclusive + assignedId, err = strconv.Atoi(fields[1][1:]) + } else if fields[1] != "" { + // Fixed mapping + mapping = Fixed + assignedId, err = strconv.Atoi(fields[1]) + } + } + + // Third column, SPD file override + if len(fields) >= 3 { + if len(fields[2]) == 0 { + err = fmt.Errorf("mem_parts_used_file file is incorrectly formatted, SPD file column is empty") + } else { + spdOverride = fields[2] + } }
if err != nil { return nil, err } - if assignedId > MaxMemoryId || assignedId < 0 { + + if assignedId > MaxMemoryId { return nil, fmt.Errorf("Out of bounds assigned id %d for part %s", assignedId, fields[0]) } - parts = append(parts, usedPart{fields[0], assignedId, mapping}) - } else { - return nil, fmt.Errorf("mem_parts_used_file file is incorrectly formatted") + + parts = append(parts, usedPart{fields[0], assignedId, mapping, spdOverride}) } }
@@ -288,7 +303,6 @@
// Assign parts with fixed ids first for _, p := range parts { - if p.index == -1 { continue } @@ -299,7 +313,7 @@
SPDFileName, ok := partToSPDMap[p.partName] if !ok { - return nil, fmt.Errorf("Failed to find part ", p.partName, " in SPD Manifest. Please add the part to global part list and regenerate SPD Manifest") + return nil, fmt.Errorf("Failed to find part %s in SPD Manifest. Please add the part to global part list and regenerate SPD Manifest", p.partName) }
// Extend partIdList and assignedMapping with empty entries if needed @@ -396,7 +410,7 @@ * This function generates Makefile.inc under the variant directory path and adds assigned SPDs * to SPD_SOURCES. */ -func genMakefile(partIdList []partIds, makefileDirName string, SPDDir string) error { +func genMakefile(partIdList []partIds, makefileDirName string, SPDDir string, partsDir string) error { s := getFileHeader() s += fmt.Sprintf("SPD_SOURCES =\n")
@@ -405,7 +419,18 @@ s += fmt.Sprintf("SPD_SOURCES += %v ", filepath.Join(SPDDir, SPDEmptyFileName)) s += fmt.Sprintf(" # ID = %d(0b%04b)\n", i, int64(i)) } else { - s += fmt.Sprintf("SPD_SOURCES += %v ", filepath.Join(SPDDir, partIdList[i].SPDFileName)) + SPDFileName := partIdList[i].SPDFileName + path := filepath.Join(partsDir, SPDFileName) + + // Check if the file exists in the directory of the parts file + if _, err := os.Stat(path); err != nil { + // File doesn't exist, check spd directory + path = filepath.Join(SPDDir, SPDFileName) + if _, err = os.Stat(path); err != nil { + return fmt.Errorf("Failed to write Makefile, SPD file '%s' doesn't exist", SPDFileName) + } + } + s += fmt.Sprintf("SPD_SOURCES += %v ", path) s += fmt.Sprintf(" # ID = %d(0b%04b) ", i, int64(i)) s += fmt.Sprintf(" Parts = %04s\n", partIdList[i].memParts) } @@ -442,12 +467,20 @@ log.Fatal(err) }
+ // Update our SPD maps with part specific overrides + for _, p := range parts { + if p.SPDOverride != "" { + partToSPDMap[p.partName] = p.SPDOverride + SPDToIndexMap[p.SPDOverride] = -1 + } + } + partIdList, err := genPartIdInfo(parts, partToSPDMap, SPDToIndexMap, makefileDir) if err != nil { log.Fatal(err) }
- if err := genMakefile(partIdList, makefileDir, SPDDir); err != nil { + if err := genMakefile(partIdList, makefileDir, SPDDir, filepath.Dir(memPartsUsedFile)); err != nil { log.Fatal(err) } }