Change in em100[master]: Simple tool to parse flash reads using em100 trace

[Stefan Reinauer (Code Review)]

Hello Furquan Shaikh,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/em100/+/48548

to review the following change.

Change subject: Simple tool to parse flash reads using em100 trace
......................................................................

Simple tool to parse flash reads using em100 trace

Signed-off-by: Furquan Shaikh 
Signed-off-by: Stefan Reinauer 
Change-Id: Id9c44760237cb4517acab8cee3119dd65801a9f7
---
A parse_em100.go
1 file changed, 202 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/em100 refs/changes/48/48548/1

diff --git a/parse_em100.go b/parse_em100.go
new file mode 100644
index 0000000..5cb7bec
--- /dev/null
+++ b/parse_em100.go
@@ -0,0 +1,202 @@
+
+package main
+
+import (
+	"bufio"
+	"fmt"
+	"os"
+	"regexp"
+	"strconv"
+	"strings"
+)
+
+type mapEntry struct {
+	startAddr uint64
+	endAddr   uint64
+	length    uint64
+	desc      string
+}
+
+func isHexNumber(s string) bool {
+	_, err := strconv.ParseInt(s, 16, 32)
+	return (err == nil)
+}
+
+func readLines(filename string) []string {
+
+	file, err := os.Open(filename)
+	if (err != nil) {
+		fmt.Println("Failed to open", filename)
+		os.Exit(1)
+	}
+
+	defer file.Close()
+
+	scanner := bufio.NewScanner(file)
+	scanner.Split(bufio.ScanLines)
+
+	result := []string{}
+
+	for scanner.Scan() {
+		line := scanner.Text()
+		result = append(result, line)
+	}
+
+	return result
+}
+
+func parseMap(mapFile string) []mapEntry {
+	lines := readLines(mapFile)
+	space := regexp.MustCompile(`\s+`)
+	mapTable := []mapEntry{}
+
+	for _,l := range lines {
+		l = space.ReplaceAllString(l, " ")
+
+		w := strings.SplitN(l, " ", 4)
+
+		if !isHexNumber(w[0]) {
+			continue
+		}
+
+		me := mapEntry{}
+		me.startAddr,_ = strconv.ParseUint(w[0], 16, 32)
+		me.endAddr,_ = strconv.ParseUint(w[1], 16, 32)
+		me.length,_ = strconv.ParseUint(w[2], 16, 32)
+		me.desc = w[3]
+
+		mapTable = append(mapTable, me)
+	}
+
+	return mapTable
+}
+
+func isReadOp(s string) bool {
+	n, err := strconv.ParseInt(s, 0, 32)
+
+	if err != nil {
+		return false
+	}
+
+	if n == 0xb || n == 0x3 {
+		return true
+	}
+
+	return false
+}
+
+func getMapEntry(addr uint64, fitMap []mapEntry) *mapEntry {
+	var len uint64
+	var result *mapEntry
+
+	len = 0
+	result = nil
+
+	for i,e := range fitMap {
+		if addr >= e.startAddr && addr <= e.endAddr {
+			if len == 0 || (len > 0 && e.length < len) {
+				len = e.length
+				result = &fitMap[i]
+			}
+		}
+
+		if addr < e.startAddr {
+			break
+		}
+	}
+
+	return result
+}
+
+func readType(op string) string {
+	n,_ := strconv.ParseInt(op, 0, 32)
+	if n == 0xb {
+		return "Fast"
+	}
+	return "Slow"
+}
+
+func getLast(s int, l int) string {
+	if s == l {
+		return ""
+	}
+	return fmt.Sprintf(":0x%x", l)
+}
+
+func appendNewEntry(result []string, e *mapEntry, startAddr int, lastAddr int, op string) []string {
+
+	if startAddr == -1 || lastAddr == -1 || e == nil {
+		return result
+	}
+
+	return append(result, fmt.Sprintf("-----> %s read [0x%x%s] from %s", readType(op), startAddr, getLast(startAddr, lastAddr), e.desc))
+
+//	return append(result, fmt.Sprintf("-----> Region %s[0x%x:0x%x]: read command from 0x%x to 0x%x",
+//		e.desc, e.startAddr, e.endAddr, startAddr, lastAddr))
+}
+
+func parseLogs(logFile string, fitMap []mapEntry) []string {
+
+	var e *mapEntry
+
+	lines := readLines(logFile)
+	space := regexp.MustCompile(`\s+`)
+
+	result := []string{}
+	e = nil
+	startAddr := -1
+	lastAddr := -1
+
+	for _,l := range lines {
+		l = space.ReplaceAllString(l, " ")
+
+		if l == "" {
+			continue
+		}
+
+		w := strings.SplitN(l, " ", 4)
+
+		if !isReadOp(w[0]) {
+			result = appendNewEntry(result[0:], e, startAddr, lastAddr, w[0])
+			startAddr = -1
+			lastAddr = -1
+			e = nil
+			result = append(result, l)
+			continue
+		}
+
+		currAddr,_ := strconv.ParseUint(w[2], 0, 32)
+		m := getMapEntry(currAddr, fitMap)
+
+		if e != m {
+			result = appendNewEntry(result[0:], e, startAddr, lastAddr, w[0])
+			startAddr = int(currAddr)
+		}
+
+		if m == nil {
+			s := fmt.Sprintf("Address out of bounds:", currAddr)
+			result = append(result, s)
+		}
+
+		e = m
+		lastAddr = int(currAddr)
+	}
+
+	return result
+}
+
+func main() {
+	argCount := len(os.Args[1:])
+
+	if (argCount != 2) {
+		fmt.Println("Usage: ", os.Args[0], "<Map File> <Log file>")
+		os.Exit(1)
+	}
+
+	fitMap := parseMap(os.Args[1])
+
+	result := parseLogs(os.Args[2], fitMap)
+	for _, l := range result {
+		fmt.Println(l)
+	}
+}

[Stefan Reinauer (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/em100/+/48548

to look at the new patch set (#2).

Change subject: Simple tool to parse flash reads using em100 trace
......................................................................

Simple tool to parse flash reads using em100 trace

Similar to the built-in functionality, this tool lets
you analyze which sections of the SPI flash are accessed
during boot.

This also includes a sample map

Signed-off-by: Furquan Shaikh 
Change-Id: Id9c44760237cb4517acab8cee3119dd65801a9f7
---
A parse_em100.go
A sample_map.txt
2 files changed, 251 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/em100 refs/changes/48/48548/2

[Stefan Reinauer (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/em100/+/48548

to look at the new patch set (#3).

Change subject: Simple tool to parse flash reads using em100 trace
......................................................................

Simple tool to parse flash reads using em100 trace

Similar to the built-in functionality, this tool lets
you analyze which sections of the SPI flash are accessed
during boot.

This also includes a sample map

Signed-off-by: Furquan Shaikh 
Change-Id: Id9c44760237cb4517acab8cee3119dd65801a9f7
---
A parse_em100.go
A sample_map.txt
2 files changed, 261 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/em100 refs/changes/48/48548/3

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/em100/+/48548 )

Change subject: Simple tool to parse flash reads using em100 trace
......................................................................

Patch Set 3:

Since we are integrating the support in em100 directly, do you still see value in having a separate tool? Is this mostly for offline analysis in that case?

[Stefan Reinauer (Code Review)]

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/em100/+/48548 )

Change subject: Simple tool to parse flash reads using em100 trace
......................................................................

Patch Set 3:
> Patch Set 3:
>
> Since we are integrating the support in em100 directly, do you still see value in having a separate tool? Is this mostly for offline analysis in that case?

I've been leaving this here for reference to get some input whether folks prefer this or the builtin version. I can also drop the builtin version. The nice thing about this version is it supports grouping the writes.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/em100/+/48548 )

Change subject: Simple tool to parse flash reads using em100 trace
......................................................................

Patch Set 3:
> Patch Set 3:
> > Patch Set 3:
> >
> > Since we are integrating the support in em100 directly, do you still see value in having a separate tool? Is this mostly for offline analysis in that case?
>
> I've been leaving this here for reference to get some input whether folks prefer this or the builtin version. I can also drop the builtin version. The nice thing about this version is it supports grouping the writes.

I posted some comments on the builtin version to see if we can add the functionality to group the reads/writes in there. Let me know what you think.