Change in ...coreboot[master]: AGESA binaryPI: Add AGESA entry timestamps

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31515

Change subject: AGESA binaryPI: Add AGESA entry timestamps
......................................................................

AGESA binaryPI: Add AGESA entry timestamps

Change-Id: I71e09d3bc4c8657979d447b90fb6ac7cae959479
Signed-off-by: Kyösti Mälkki 
---
M src/drivers/amd/agesa/eventlog.c
M src/drivers/amd/agesa/state_machine.c
M src/northbridge/amd/agesa/state_machine.h
3 files changed, 37 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/31515/1

diff --git a/src/drivers/amd/agesa/eventlog.c b/src/drivers/amd/agesa/eventlog.c
index 152011e..887da30 100644
--- a/src/drivers/amd/agesa/eventlog.c
+++ b/src/drivers/amd/agesa/eventlog.c
@@ -16,6 +16,7 @@
 #include <console/console.h>
 #include <stdint.h>
 #include <string.h>
+#include <timestamp.h>

#include <northbridge/amd/agesa/state_machine.h>
 #include <northbridge/amd/agesa/BiosCallOuts.h>
@@ -30,54 +31,78 @@
 {
    AGESA_STRUCT_NAME func;
    const char *name;
+	uint32_t entry_id;
+	uint32_t exit_id;
 };

static const struct agesa_mapping entrypoint[] = {
    {
    	.func = AMD_INIT_RESET,
    	.name = "AmdInitReset",
+		.entry_id = TS_AGESA_INIT_RESET_START,
+		.exit_id = TS_AGESA_INIT_RESET_DONE,
    },
    {
    	.func = AMD_INIT_EARLY,
    	.name = "AmdInitEarly",
+		.entry_id = TS_AGESA_INIT_EARLY_START,
+		.exit_id = TS_AGESA_INIT_EARLY_DONE,
    },
    {
    	.func = AMD_INIT_POST,
    	.name = "AmdInitPost",
+		.entry_id = TS_AGESA_INIT_POST_START,
+		.exit_id = TS_AGESA_INIT_POST_DONE,
    },
    {
    	.func = AMD_INIT_RESUME,
    	.name = "AmdInitResume",
+		.entry_id = TS_AGESA_INIT_RESUME_START,
+		.exit_id = TS_AGESA_INIT_RESUME_DONE,
    },
    {
    	.func = AMD_INIT_ENV,
    	.name = "AmdInitEnv",
+		.entry_id = TS_AGESA_INIT_ENV_START,
+		.exit_id = TS_AGESA_INIT_ENV_DONE,
    },
    {
    	.func = AMD_INIT_MID,
    	.name = "AmdInitMid",
+		.entry_id = TS_AGESA_INIT_MID_START,
+		.exit_id = TS_AGESA_INIT_MID_DONE,
    },
    {
    	.func = AMD_INIT_LATE,
    	.name = "AmdInitLate",
+		.entry_id = TS_AGESA_INIT_LATE_START,
+		.exit_id = TS_AGESA_INIT_LATE_DONE,
    },
    {
    	.func = AMD_S3LATE_RESTORE,
    	.name = "AmdS3LateRestore",
+		.entry_id = TS_AGESA_S3_LATE_START,
+		.exit_id = TS_AGESA_S3_LATE_DONE,
    },
 #if !defined(AMD_S3_SAVE_REMOVED)
    {
    	.func = AMD_S3_SAVE,
    	.name = "AmdS3Save",
+		.entry_id = TS_AGESA_INIT_RTB_START,
+		.exit_id = TS_AGESA_INIT_RTB_DONE,
    },
 #endif
    {
    	.func = AMD_S3FINAL_RESTORE,
    	.name = "AmdS3FinalRestore",
+		.entry_id = TS_AGESA_S3_FINAL_START,
+		.exit_id = TS_AGESA_S3_FINAL_DONE,
    },
    {
    	.func = AMD_INIT_RTB,
    	.name = "AmdInitRtb",
+		.entry_id = TS_AGESA_INIT_RTB_START,
+		.exit_id = TS_AGESA_INIT_RTB_DONE,
    },
 };

@@ -92,6 +117,8 @@
    for (i = 0; i < ARRAY_SIZE(entrypoint); i++) {
    	if (task->func == entrypoint[i].func) {
    		task->function_name = entrypoint[i].name;
+			task->ts_entry_id = entrypoint[i].entry_id;
+			task->ts_exit_id = entrypoint[i].exit_id;
    		break;
    	}
    }
diff --git a/src/drivers/amd/agesa/state_machine.c b/src/drivers/amd/agesa/state_machine.c
index f676192..4d56218 100644
--- a/src/drivers/amd/agesa/state_machine.c
+++ b/src/drivers/amd/agesa/state_machine.c
@@ -21,6 +21,8 @@
 #include <arch/cpu.h>
 #include <bootstate.h>
 #include <cbfs.h>
+#include <cbmem.h>
+#include <timestamp.h>

#include <northbridge/amd/agesa/state_machine.h>
 #include <northbridge/amd/agesa/agesa_helper.h>
@@ -263,12 +265,18 @@
    AMD_CONFIG_PARAMS *StdHeader = aip.NewStructPtr;
    ASSERT(StdHeader->Func == func);

+	if (task.ts_entry_id)
+		timestamp_add_now(task.ts_entry_id);
+
    if (ENV_ROMSTAGE)
    	final = romstage_dispatch(cb, func, StdHeader);

if (ENV_RAMSTAGE)
    	final = ramstage_dispatch(cb, func, StdHeader);

+	if (task.ts_exit_id)
+		timestamp_add_now(task.ts_exit_id);
+
    agesawrapper_trace(final, StdHeader, task.function_name);
    ASSERT(final < AGESA_FATAL);

diff --git a/src/northbridge/amd/agesa/state_machine.h b/src/northbridge/amd/agesa/state_machine.h
index 45d57c2..f1dd139 100644
--- a/src/northbridge/amd/agesa/state_machine.h
+++ b/src/northbridge/amd/agesa/state_machine.h
@@ -60,6 +60,8 @@

AGESA_STRUCT_NAME func;
    const char *function_name;
+	uint32_t ts_entry_id;
+	uint32_t ts_exit_id;
 };

void agesa_state_on_entry(struct agesa_state *task, AGESA_STRUCT_NAME func);

[Kyösti Mälkki (Code Review)]

Hello build bot (Jenkins),

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

https://review.coreboot.org/c/coreboot/+/31515

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

Change subject: AGESA binaryPI: Add AGESA entry timestamps
......................................................................

AGESA binaryPI: Add AGESA entry timestamps

The call to timestamp_rescale_table() had to be moved
before TS_AGESA_INIT_{POST/RESUME}_DONE to have that
timestamp appear without rescaling.

Change-Id: I71e09d3bc4c8657979d447b90fb6ac7cae959479
Signed-off-by: Kyösti Mälkki 
---
M src/drivers/amd/agesa/eventlog.c
M src/drivers/amd/agesa/romstage.c
M src/drivers/amd/agesa/state_machine.c
M src/northbridge/amd/agesa/state_machine.h
M src/vendorcode/amd/Kconfig
5 files changed, 56 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/31515/4

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31515 )

Change subject: AGESA binaryPI: Add AGESA entry timestamps
......................................................................

Patch Set 4: Code-Review+2

[Kyösti Mälkki (Code Review)]

Hello Marshall Dawson, build bot (Jenkins),

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

https://review.coreboot.org/c/coreboot/+/31515

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

Change subject: AGESA binaryPI: Add AGESA entry timestamps
......................................................................

AGESA binaryPI: Add AGESA entry timestamps

The call to timestamp_rescale_table() had to be moved
before TS_AGESA_INIT_{POST/RESUME}_DONE to have that
timestamp appear without rescaling.

Change-Id: I71e09d3bc4c8657979d447b90fb6ac7cae959479
Signed-off-by: Kyösti Mälkki 
---
M src/drivers/amd/agesa/eventlog.c
M src/drivers/amd/agesa/romstage.c
M src/drivers/amd/agesa/state_machine.c
M src/northbridge/amd/agesa/state_machine.h
M src/vendorcode/amd/Kconfig
5 files changed, 57 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/31515/5

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31515 )

Change subject: AGESA binaryPI: Add AGESA entry timestamps
......................................................................

Patch Set 5: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31515 )

Change subject: AGESA binaryPI: Add AGESA entry timestamps
......................................................................

AGESA binaryPI: Add AGESA entry timestamps

The call to timestamp_rescale_table() had to be moved
before TS_AGESA_INIT_{POST/RESUME}_DONE to have that
timestamp appear without rescaling.

Change-Id: I71e09d3bc4c8657979d447b90fb6ac7cae959479
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31515
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Georgi 
---
M src/drivers/amd/agesa/eventlog.c
M src/drivers/amd/agesa/romstage.c
M src/drivers/amd/agesa/state_machine.c
M src/northbridge/amd/agesa/state_machine.h
M src/vendorcode/amd/Kconfig
5 files changed, 57 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved

diff --git a/src/drivers/amd/agesa/eventlog.c b/src/drivers/amd/agesa/eventlog.c
index 152011e..887da30 100644
--- a/src/drivers/amd/agesa/eventlog.c
+++ b/src/drivers/amd/agesa/eventlog.c
@@ -16,6 +16,7 @@
 #include <console/console.h>
 #include <stdint.h>
 #include <string.h>
+#include <timestamp.h>

#include <northbridge/amd/agesa/state_machine.h>
 #include <northbridge/amd/agesa/BiosCallOuts.h>
@@ -30,54 +31,78 @@
 {
    AGESA_STRUCT_NAME func;
    const char *name;
+	uint32_t entry_id;
+	uint32_t exit_id;
 };

static const struct agesa_mapping entrypoint[] = {
    {
    	.func = AMD_INIT_RESET,
    	.name = "AmdInitReset",
+		.entry_id = TS_AGESA_INIT_RESET_START,
+		.exit_id = TS_AGESA_INIT_RESET_DONE,
    },
    {
    	.func = AMD_INIT_EARLY,
    	.name = "AmdInitEarly",
+		.entry_id = TS_AGESA_INIT_EARLY_START,
+		.exit_id = TS_AGESA_INIT_EARLY_DONE,
    },
    {
    	.func = AMD_INIT_POST,
    	.name = "AmdInitPost",
+		.entry_id = TS_AGESA_INIT_POST_START,
+		.exit_id = TS_AGESA_INIT_POST_DONE,
    },
    {
    	.func = AMD_INIT_RESUME,
    	.name = "AmdInitResume",
+		.entry_id = TS_AGESA_INIT_RESUME_START,
+		.exit_id = TS_AGESA_INIT_RESUME_DONE,
    },
    {
    	.func = AMD_INIT_ENV,
    	.name = "AmdInitEnv",
+		.entry_id = TS_AGESA_INIT_ENV_START,
+		.exit_id = TS_AGESA_INIT_ENV_DONE,
    },
    {
    	.func = AMD_INIT_MID,
    	.name = "AmdInitMid",
+		.entry_id = TS_AGESA_INIT_MID_START,
+		.exit_id = TS_AGESA_INIT_MID_DONE,
    },
    {
    	.func = AMD_INIT_LATE,
    	.name = "AmdInitLate",
+		.entry_id = TS_AGESA_INIT_LATE_START,
+		.exit_id = TS_AGESA_INIT_LATE_DONE,
    },
    {
    	.func = AMD_S3LATE_RESTORE,
    	.name = "AmdS3LateRestore",
+		.entry_id = TS_AGESA_S3_LATE_START,
+		.exit_id = TS_AGESA_S3_LATE_DONE,
    },
 #if !defined(AMD_S3_SAVE_REMOVED)
    {
    	.func = AMD_S3_SAVE,
    	.name = "AmdS3Save",
+		.entry_id = TS_AGESA_INIT_RTB_START,
+		.exit_id = TS_AGESA_INIT_RTB_DONE,
    },
 #endif
    {
    	.func = AMD_S3FINAL_RESTORE,
    	.name = "AmdS3FinalRestore",
+		.entry_id = TS_AGESA_S3_FINAL_START,
+		.exit_id = TS_AGESA_S3_FINAL_DONE,
    },
    {
    	.func = AMD_INIT_RTB,
    	.name = "AmdInitRtb",
+		.entry_id = TS_AGESA_INIT_RTB_START,
+		.exit_id = TS_AGESA_INIT_RTB_DONE,
    },
 };

@@ -92,6 +117,8 @@
    for (i = 0; i < ARRAY_SIZE(entrypoint); i++) {
    	if (task->func == entrypoint[i].func) {
    		task->function_name = entrypoint[i].name;
+			task->ts_entry_id = entrypoint[i].entry_id;
+			task->ts_exit_id = entrypoint[i].exit_id;
    		break;
    	}
    }
diff --git a/src/drivers/amd/agesa/romstage.c b/src/drivers/amd/agesa/romstage.c
index d5b20b7..adf6e0d 100644
--- a/src/drivers/amd/agesa/romstage.c
+++ b/src/drivers/amd/agesa/romstage.c
@@ -90,8 +90,6 @@
    else
    	agesa_execute_state(cb, AMD_INIT_RESUME);

-	/* FIXME: Detect if TSC frequency changed during raminit? */
-	timestamp_rescale_table(1, 4);
    timestamp_add_now(TS_AFTER_INITRAM);

/* Work around AGESA setting all memory as WB on normal
diff --git a/src/drivers/amd/agesa/state_machine.c b/src/drivers/amd/agesa/state_machine.c
index 03c6582..c8529c5e 100644
--- a/src/drivers/amd/agesa/state_machine.c
+++ b/src/drivers/amd/agesa/state_machine.c
@@ -20,7 +20,9 @@
 #include <arch/cpu.h>
 #include <bootstate.h>
 #include <cbfs.h>
-#include <console/console.h>
+#include <cbmem.h>
+#include <timestamp.h>
+
 #include <northbridge/amd/agesa/state_machine.h>
 #include <northbridge/amd/agesa/agesa_helper.h>
 #include <northbridge/amd/agesa/BiosCallOuts.h>
@@ -147,6 +149,11 @@
    		platform_BeforeInitPost(cb, param);
    		board_BeforeInitPost(cb, param);
    		status = module_dispatch(func, StdHeader);
+
+			/* FIXME: Detect if TSC frequency really
+			 * changed during raminit? */
+			timestamp_rescale_table(1, 4);
+
    		platform_AfterInitPost(cb, param);
    		break;
    	}
@@ -156,6 +163,11 @@
    		AMD_RESUME_PARAMS *param = (void *)StdHeader;
    		platform_BeforeInitResume(cb, param);
    		status = module_dispatch(func, StdHeader);
+
+			/* FIXME: Detect if TSC frequency really
+			 * changed during raminit? */
+			timestamp_rescale_table(1, 4);
+
    		platform_AfterInitResume(cb, param);
    		break;
    	}
@@ -262,12 +274,18 @@
    AMD_CONFIG_PARAMS *StdHeader = aip.NewStructPtr;
    ASSERT(StdHeader->Func == func);

+	if (CONFIG(AGESA_EXTRA_TIMESTAMPS) && task.ts_entry_id)
+		timestamp_add_now(task.ts_entry_id);
+
    if (ENV_ROMSTAGE)
    	final = romstage_dispatch(cb, func, StdHeader);

if (ENV_RAMSTAGE)
    	final = ramstage_dispatch(cb, func, StdHeader);

+	if (CONFIG(AGESA_EXTRA_TIMESTAMPS) && task.ts_exit_id)
+		timestamp_add_now(task.ts_exit_id);
+
    agesawrapper_trace(final, StdHeader, task.function_name);
    ASSERT(final < AGESA_FATAL);

diff --git a/src/northbridge/amd/agesa/state_machine.h b/src/northbridge/amd/agesa/state_machine.h
index 74c3f61..7d9fe9c 100644
--- a/src/northbridge/amd/agesa/state_machine.h
+++ b/src/northbridge/amd/agesa/state_machine.h
@@ -59,6 +59,8 @@

AGESA_STRUCT_NAME func;
    const char *function_name;
+	uint32_t ts_entry_id;
+	uint32_t ts_exit_id;
 };

void agesa_state_on_entry(struct agesa_state *task, AGESA_STRUCT_NAME func);
diff --git a/src/vendorcode/amd/Kconfig b/src/vendorcode/amd/Kconfig
index 3fe0c82..44b3940 100644
--- a/src/vendorcode/amd/Kconfig
+++ b/src/vendorcode/amd/Kconfig
@@ -47,6 +47,15 @@
 source src/vendorcode/amd/pi/Kconfig
 endif

+config AGESA_EXTRA_TIMESTAMPS
+	bool "Add instrumentation for AGESA calls"
+	default n
+	depends on !BINARYPI_LEGACY_WRAPPER
+	depends on DRIVERS_AMD_PI
+	help
+	  Insert additional timestamps around each entrypoint into
+	  AGESA vendorcode.
+
 endmenu

endif