Change in coreboot[master]: util/spd_tools: Add ability to override SPD file for parts

[Felix Held (Code Review)]

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 
Change-Id: I078dd04fead2bf19f53bc6ca8295187d439adc20
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63281
Tested-by: build bot (Jenkins) 
Reviewed-by: Rob Barnes 
---
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)
    }
 }