Robert Zieba has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/62905 )
Change subject: util/spd_tools: Add support for exclusive IDs ......................................................................
util/spd_tools: Add support for exclusive IDs
Currently memory parts that use the same SPD are assigned the same ID by spd_tools. This commit adds support for exclusive IDs. When given an exclusive ID a memory part will not share its ID with other parts unless they have also have the same exclusive ID.
BUG=b:225161910 TEST=Ran part_id_gen and checked that exclusive IDs work correctly and that the current behavior still works in their abscence.
Signed-off-by: Robert Zieba robertzieba@google.com Change-Id: Ife5afe32337f69bc06451ce16238c7a83bc983c8 --- M util/spd_tools/README.md M util/spd_tools/src/part_id_gen/part_id_gen.go 2 files changed, 56 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/62905/1
diff --git a/util/spd_tools/README.md b/util/spd_tools/README.md index 3a1342c..01bc417 100644 --- a/util/spd_tools/README.md +++ b/util/spd_tools/README.md @@ -459,10 +459,14 @@ * 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 ID for each part. NOTE: Only assign a fixed ID if required - for legacy reasons. +* 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. +* NOTE: Only assign a fixed/exclusive ID if required for legacy reasons.
-Example of a CSV file using fixed IDs: +Example of a CSV file using fixed and exclusive IDs:
``` K4AAG165WA-BCWE,1 @@ -470,13 +474,15 @@ MT40A1G16KD-062E:E K4A8G165WC-BCWE H5AN8G6NDJR-XNC,8 -H5ANAG6NCMR-XNC +H5ANAG6NCMR-XNC,*9 ```
Explanation: This will ensure that the SPDs for K4AAG165WA-BCWE and -H5AN8G6NDJR-XNC are assigned to IDs 1 and 8 respectively. The SPDs for all other -memory parts will be assigned to the first compatible ID. Assigning fixed IDs -may result in duplicate SPD entries or gaps in the ID mapping. +H5AN8G6NDJR-XNC are assigned to IDs 1 and 8 respectively. H5ANAG6NCMR-XNC +will be assigned ID 9 and no other part will be assigned ID 9 even if it +shares the same SPD. The SPDs for all other memory parts will be assigned to +the first compatible ID. Assigning fixed/exclusive IDs may result in duplicate +SPD entries or gaps in the ID mapping.
### Output
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 750b825..73f71ac 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 @@ -93,9 +93,17 @@ return nil }
+type mappingType int + +const ( + Normal mappingType = iota + Exclusive +) + type usedPart struct { partName string index int + mapping mappingType }
func readPlatformsManifest(memTech string) (map[string]string, error) { @@ -174,16 +182,27 @@ }
if len(fields) == 1 { - parts = append(parts, usedPart{fields[0], -1}) + parts = append(parts, usedPart{fields[0], -1, Normal}) } else if len(fields) == 2 { - assignedId, err := strconv.Atoi(fields[1]) + var mapping = Normal + var assignedId = -1 + var err error = nil + + if len(fields[1]) >= 2 && fields[1][0] == '*' { + // Exclusive mapping + mapping = Exclusive + assignedId, err = strconv.Atoi(fields[1][1:]) + } else { + assignedId, err = strconv.Atoi(fields[1]) + } + if err != nil { return nil, err } if assignedId > MaxMemoryId || assignedId < 0 { return nil, fmt.Errorf("Out of bounds assigned id %d for part %s", assignedId, fields[0]) } - parts = append(parts, usedPart{fields[0], assignedId}) + parts = append(parts, usedPart{fields[0], assignedId, mapping}) } else { return nil, fmt.Errorf("mem_parts_used_file file is incorrectly formatted") } @@ -245,7 +264,7 @@ }
func getFileHeader() string { - return `# SPDX-License-Identifier: GPL-2.0-or-later + return `# SPDX-License-Identifier: GPL-2.0-or-later # This is an auto-generated file. Do not edit!! # Generated by: ` + fmt.Sprintf("# %s\n\n", strings.Join(os.Args[0:], " ")) @@ -262,6 +281,7 @@ */ func genPartIdInfo(parts []usedPart, partToSPDMap map[string]string, SPDToIndexMap map[string]int, makefileDirName string) ([]partIds, error) { partIdList := []partIds{} + assignedMapping := map[int]mappingType{} var s string
// Assign parts with fixed ids first @@ -285,11 +305,26 @@ partIdList = append(partIdList, partIds{}) }
- if partIdList[p.index].SPDFileName != "" { - return nil, fmt.Errorf("Part ", p.partName, " is assigned to an already assigned ID ", p.index) + // Only allow parts with the same index if they share the same SPD + assignedSPD := partIdList[p.index].SPDFileName + if assignedSPD != "" && assignedSPD != partToSPDMap[p.partName] { + return nil, fmt.Errorf("ID %d is already assigned to %s, conflicting with %s(%s)", p.index, assignedSPD, p.partName, SPDFileName) }
- partIdList[p.index] = partIds{SPDFileName: SPDFileName, memParts: p.partName} + if mapping, present := assignedMapping[p.index]; present { + // Don't allow explicit and non-explict parts to use the same index + if mapping != p.mapping && (mapping == Exclusive || p.mapping == Exclusive) { + return nil, fmt.Errorf("Exclusive/non-exclusive conflict in assigning %s to ID %d", p.partName, p.index) + } + } else { + assignedMapping[p.index] = p.mapping + } + + if partIdList[p.index].memParts == "" { + partIdList[p.index] = partIds{SPDFileName: SPDFileName, memParts: p.partName} + } else { + partIdList[p.index].memParts += ", " + p.partName + }
// SPDToIndexMap should point to first assigned index in the used part list if SPDToIndexMap[SPDFileName] < 0 { @@ -317,7 +352,7 @@ }
index := SPDToIndexMap[SPDFileName] - if index != -1 { + if index != -1 && assignedMapping[index] != Exclusive { partIdList[index].memParts += ", " + p.partName appendPartIdInfo(&s, p.partName, index) continue