[coreboot-gerrit] Change in coreboot[master]: cbmem: Sort timestamp entries

[Philipp Deppenwiese (Code Review)]

Philipp Deppenwiese has submitted this change and it was merged. ( https://review.coreboot.org/26168 )

Change subject: cbmem: Sort timestamp entries
......................................................................

cbmem: Sort timestamp entries

If the timestamp entries are added out of order, the duration
calculation will be wrong.

AGESA collects timestamp data through all the stages. Then in AmdInitPost
it asks for a buffer to write TP_Perf_STRUCT into. agesawrapper will then
take the data and call timestamp_add on each entry. This results in
the entries being out of order.

TEST=Built firmware for grunt that manually added entries and then ran
cbmem -t/-T to verify the entries were in the correct order.

Change-Id: I6946a844b71d714141b3372e4c43807cfe3528ad
Signed-off-by: Raul E Rangel <rrangel at chromium.org>
Reviewed-on: https://review.coreboot.org/26168
Reviewed-by: Philipp Deppenwiese <zaolin.daisuki at gmail.com>
Reviewed-by: Martin Roth <martinroth at google.com>
Tested-by: build bot (Jenkins) <no-reply at coreboot.org>
---
M util/cbmem/cbmem.c
1 file changed, 21 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Martin Roth: Looks good to me, approved
  Philipp Deppenwiese: Looks good to me, approved



diff --git a/util/cbmem/cbmem.c b/util/cbmem/cbmem.c
index eae5da3..21da71b 100644
--- a/util/cbmem/cbmem.c
+++ b/util/cbmem/cbmem.c
@@ -584,11 +584,20 @@
 	return step_time;
 }
 
+static int compare_timestamp_entries(const void *a, const void *b)
+{
+	const struct timestamp_entry *tse_a = (struct timestamp_entry *)a;
+	const struct timestamp_entry *tse_b = (struct timestamp_entry *)b;
+
+	return tse_a->entry_stamp - tse_b->entry_stamp;
+}
+
 /* dump the timestamp table */
 static void dump_timestamps(int mach_readable)
 {
 	int i;
 	const struct timestamp_table *tst_p;
+	struct timestamp_table *sorted_tst_p;
 	size_t size;
 	uint64_t prev_stamp;
 	uint64_t total_time;
@@ -625,13 +634,21 @@
 		timestamp_print_entry(0,  tst_p->base_time, prev_stamp);
 	prev_stamp = tst_p->base_time;
 
+	sorted_tst_p = malloc(size);
+	if (!sorted_tst_p)
+		die("Failed to allocate memory");
+	memcpy(sorted_tst_p, tst_p, size);
+
+	qsort(&sorted_tst_p->entries[0], sorted_tst_p->num_entries,
+	      sizeof(struct timestamp_entry), compare_timestamp_entries);
+
 	total_time = 0;
-	for (i = 0; i < tst_p->num_entries; i++) {
+	for (i = 0; i < sorted_tst_p->num_entries; i++) {
 		uint64_t stamp;
-		const struct timestamp_entry *tse = &tst_p->entries[i];
+		const struct timestamp_entry *tse = &sorted_tst_p->entries[i];
 
 		/* Make all timestamps absolute. */
-		stamp = tse->entry_stamp + tst_p->base_time;
+		stamp = tse->entry_stamp + sorted_tst_p->base_time;
 		if (mach_readable)
 			total_time +=
 				timestamp_print_parseable_entry(tse->entry_id,
@@ -649,6 +666,7 @@
 	}
 
 	unmap_memory(&timestamp_mapping);
+	free(sorted_tst_p);
 }
 
 struct cbmem_console {

-- 
To view, visit https://review.coreboot.org/26168
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6946a844b71d714141b3372e4c43807cfe3528ad
Gerrit-Change-Number: 26168
Gerrit-PatchSet: 6
Gerrit-Owner: Raul Rangel <rrangel at chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki at gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel at chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180513/ed1a6ded/attachment.html>