From a662648a7fbfc04bb0f4a2ef92d7faa4fb8c7e09 Mon Sep 17 00:00:00 2001 From: Rob Barnes Date: Fri, 14 Aug 2020 09:40:04 -0600 Subject: [PATCH] util: Add support to spd_tools for fixed id For boards that have already assigned memory ids, there needs to be a way to fix parts to a specific id. After assigning all the fixed ids the tool still attempts to minimize the SPDs entries. Since a fixed ID could be anywhere, gaps can be created in the list. So an empty SPD entry is created to fill the gaps in the list until they are used. BUG=b:162939176 TEST=Generate various outputs Signed-off-by: Rob Barnes Change-Id: I1f8ea1ff4f33a97ab28ba94896a1054e89189576 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44463 Tested-by: build bot (Jenkins) Reviewed-by: Furquan Shaikh Reviewed-by: Nick Vaccaro --- util/spd_tools/ddr4/README.md | 30 +++++- util/spd_tools/ddr4/gen_part_id.go | 142 ++++++++++++++++++++++++----- util/spd_tools/ddr4/gen_spd.go | 14 ++- 3 files changed, 155 insertions(+), 31 deletions(-) diff --git a/util/spd_tools/ddr4/README.md b/util/spd_tools/ddr4/README.md index 81ab97a80a..646d937d9b 100644 --- a/util/spd_tools/ddr4/README.md +++ b/util/spd_tools/ddr4/README.md @@ -16,8 +16,8 @@ by the board. SPD files. * gen_part_id.go: Allocates DRAM strap IDs for different DDR4 - memory parts used by the board. Takes as input list of memory parts - used by the board (with one memory part on each line) and the SPD + memory parts used by the board. Takes as input a list of memory parts + used (in CSV format) by the board with optional fixed ids and the SPD manifest file generated by gen_spd.go. Generates Makefile.inc for integrating the generated SPD files in the coreboot build. @@ -192,12 +192,29 @@ This program takes as input: * Pointer to directory where the SPD files and the manifest file `ddr4_spd_manifest.generated.txt` (in CSV format) are placed by gen_spd.go -* File containing list of memory parts used by the board. Each line of - the file is supposed to contain one memory part `name` as present in - the global list of memory parts provided to gen_spd.go +* CSV file containing list of memory parts used by the board and optional + fixed id. Each line of the file is supposed to contain one memory part `name` + as present in the global list of memory parts provided to gen_spd.go. + Optionally a fixed id may also be assigned to the part if required. + NOTE: Only assign a fixed ID if required for legacy reasons. + * Pointer to directory where the generated Makefile.inc should be placed by the tool. +Sample input (mem_parts_used_file.txt): +``` +K4AAG165WA-BCWE,1 +MT40A512M16TB-062E:J +MT40A1G16KD-062E:E +K4A8G165WC-BCWE +H5AN8G6NDJR-XNC,8 +H5ANAG6NCMR-XNC +``` +NOTE: This will ensure SPDs compatible with K4AAG165WA-BCWE and H5AN8G6NDJR-XNC +are assigned to ID 1 and 8 respectively. 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. + ### Output This program provides the following: @@ -227,8 +244,11 @@ Sample Makefile.inc: SPD_SOURCES = SPD_SOURCES += ddr4-spd-1.hex # ID = 0(0b0000) Parts = MEMORY_PART_A SPD_SOURCES += ddr4-spd-2.hex # ID = 1(0b0001) Parts = MEMORY_PART_B, MEMORY_PART_D +SPD_SOURCES += ddr4-spd-empty.hex # ID = 2(0b0010) SPD_SOURCES += ddr4-spd-3.hex # ID = 2(0b0010) Parts = MEMORY_PART_C ``` +NOTE: Empty entries may be required if there is a gap created by a memory part +with a fixed id. ### Note of caution diff --git a/util/spd_tools/ddr4/gen_part_id.go b/util/spd_tools/ddr4/gen_part_id.go index e0adaaf994..c0098aba1d 100644 --- a/util/spd_tools/ddr4/gen_part_id.go +++ b/util/spd_tools/ddr4/gen_part_id.go @@ -10,7 +10,7 @@ import ( "log" "os" "path/filepath" - "strings" + "strconv" ) /* @@ -28,6 +28,7 @@ const ( SPDManifestFileName = "ddr4_spd_manifest.generated.txt" MakefileName = "Makefile.inc" DRAMIdFileName = "dram_id.generated.txt" + MaxMemoryId = 15 ) func usage() { @@ -35,7 +36,7 @@ func usage() { fmt.Printf(" where,\n") fmt.Printf(" spd_dir = Directory path containing SPD files and manifest generated by gen_spd.go\n") fmt.Printf(" makefile_dir = Directory path where generated Makefile.inc should be placed\n") - fmt.Printf(" mem_parts_used_file = File containing list of memory parts used by the board\n\n\n") + fmt.Printf(" mem_parts_used_file = CSV file containing list of memory parts used by the board and optional fixed ids\n\n\n") } func checkArgs() error { @@ -49,17 +50,53 @@ func checkArgs() error { return nil } +type usedPart struct { + partName string + index int +} /* - * Read input file that contains list of memory part names used by the variant (one on a line) - * and split into separate strings for each part name. + * Read input file CSV that contains list of memory part names used by the variant + * and an optional assigned id. */ -func readParts(memPartsUsedFileName string) ([]string, error) { - lines, err := ioutil.ReadFile(memPartsUsedFileName) +func readParts(memPartsUsedFileName string) ([]usedPart, error) { + + f, err := os.Open(memPartsUsedFileName) if err != nil { return nil, err } - str := string(lines) - parts := strings.Split(str, "\n") + defer f.Close() + r := csv.NewReader(f) + r.FieldsPerRecord = -1 // Allow variable length records + r.TrimLeadingSpace = true + + parts := []usedPart{} + + for { + fields, err := r.Read() + + if err == io.EOF { + break + } + + if err != nil { + return nil, err + } + + if len(fields) == 1 { + parts = append(parts, usedPart{fields[0], -1}) + } else if len(fields) == 2 { + 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}) + } else { + return nil, fmt.Errorf("mem_parts_used_file file is incorrectly formatted") + } + } return parts, nil } @@ -124,37 +161,87 @@ type partIds struct { * Returns list of partIds that contains spdFileName and supported memory parts for each * assigned ID. */ -func genPartIdInfo(parts []string, partToSPDMap map[string]string, SPDToIndexMap map[string]int, makefileDirName string) ([]partIds, error) { +func genPartIdInfo(parts []usedPart, partToSPDMap map[string]string, SPDToIndexMap map[string]int, makefileDirName string) ([]partIds, error) { + partIdList := []partIds{} - curId := 0 var s string - s += fmt.Sprintf("%-30s %s\n", "DRAM Part Name", "ID to assign") - + // Assign parts with fixed ids first for _, p := range parts { - if p == "" { + + if p.index == -1 { continue } - SPDFileName,ok := partToSPDMap[p] + if p.partName == "" { + return nil, fmt.Errorf("Invalid part entry") + } + + SPDFileName,ok := partToSPDMap[p.partName] if !ok { - return nil, fmt.Errorf("Failed to find part ", p, " in SPD Manifest. Please add the part to global part list and regenerate SPD Manifest") + 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") + } + + // Extend partIdList with empty entries if needed + for i := len(partIdList) - 1; i < p.index; i++ { + 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) + } + + partIdList[p.index] = partIds{SPDFileName: SPDFileName, memParts: p.partName} + + // SPDToIndexMap should point to first assigned index in the used part list + if SPDToIndexMap[SPDFileName] < 0 { + SPDToIndexMap[SPDFileName] = p.index + } + } + + s += fmt.Sprintf("%-30s %s\n", "DRAM Part Name", "ID to assign") + + // Assign parts with no fixed id + for _, p := range parts { + if p.partName == "" { + return nil, fmt.Errorf("Invalid part entry") + } + + // Add assigned parts to dram id file in the order they appear + if p.index != -1 { + appendPartIdInfo(&s, p.partName, p.index) + continue + } + + 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") } index := SPDToIndexMap[SPDFileName] if index != -1 { - partIdList[index].memParts += ", " + p - appendPartIdInfo(&s, p, index) + partIdList[index].memParts += ", " + p.partName + appendPartIdInfo(&s, p.partName, index) continue } - SPDToIndexMap[SPDFileName] = curId + // Find first empty index + for i, partId := range partIdList { + if partId.SPDFileName == "" { + index = i + break + } + } - appendPartIdInfo(&s, p, curId) - entry := partIds{SPDFileName: SPDFileName, memParts: p} - partIdList = append(partIdList, entry) + // Append new entry + if index == -1 { + index = len(partIdList) + partIdList = append(partIdList, partIds{}) + } - curId++ + SPDToIndexMap[SPDFileName] = index + appendPartIdInfo(&s, p.partName, index) + partIdList[index] = partIds{SPDFileName: SPDFileName, memParts: p.partName} } fmt.Printf("%s", s) @@ -177,9 +264,14 @@ func genMakefile(partIdList []partIds, makefileDirName string) error { s += fmt.Sprintf("SPD_SOURCES =\n") for i := 0; i < len(partIdList); i++ { - s += fmt.Sprintf("SPD_SOURCES += %s ", partIdList[i].SPDFileName) - s += fmt.Sprintf(" # ID = %d(0b%04b) ", i, int64(i)) - s += fmt.Sprintf(" Parts = %04s\n", partIdList[i].memParts) + if partIdList[i].SPDFileName == "" { + s += fmt.Sprintf("SPD_SOURCES += %s ", "ddr4-spd-empty.hex") + s += fmt.Sprintf(" # ID = %d(0b%04b)\n", i, int64(i)) + } else { + s += fmt.Sprintf("SPD_SOURCES += %s ", partIdList[i].SPDFileName) + s += fmt.Sprintf(" # ID = %d(0b%04b) ", i, int64(i)) + s += fmt.Sprintf(" Parts = %04s\n", partIdList[i].memParts) + } } return ioutil.WriteFile(filepath.Join(makefileDirName, MakefileName), []byte(s), 0644) diff --git a/util/spd_tools/ddr4/gen_spd.go b/util/spd_tools/ddr4/gen_spd.go index 99fae7d3b4..e3fa732179 100644 --- a/util/spd_tools/ddr4/gen_spd.go +++ b/util/spd_tools/ddr4/gen_spd.go @@ -968,7 +968,10 @@ func createSPD(memAttribs *memAttributes) string { var s string for i := 0; i < 512; i++ { - b := getSPDByte(i, memAttribs) + var b byte = 0 + if memAttribs != nil { + b = getSPDByte(i, memAttribs) + } if (i + 1) % 16 == 0 { s += fmt.Sprintf("%02X\n", b) @@ -997,6 +1000,13 @@ func generateSPD(memPart *memPart, SPDId int, SPDDirName string) { ioutil.WriteFile(filepath.Join(SPDDirName, memPart.SPDFileName), []byte(s), 0644) } +func generateEmptySPD(SPDDirName string) { + + s := createSPD(nil) + SPDFileName := "ddr4-spd-empty.hex" + ioutil.WriteFile(filepath.Join(SPDDirName, SPDFileName), []byte(s), 0644) +} + func readMemoryParts(memParts *memParts, memPartsFileName string) error { databytes, err := ioutil.ReadFile(memPartsFileName) if err != nil { @@ -1396,6 +1406,8 @@ func main() { } } + generateEmptySPD(SPDDir) + if err := writeSPDManifest(&memParts, SPDDir); err != nil { log.Fatal(err) }