Change in coreboot[master]: arch/x86: Obsolete CACHE_AS_RAM config

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

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

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

arch/x86: Obsolete CACHE_AS_RAM config

It was originally inverse of romcc -built romstages on x86,
and is currently always true on x86.

Change-Id: I65fa6b3ce8a86781724bbf08f5eadee4112667c4
Signed-off-by: Kyösti Mälkki 
---
M src/cpu/Kconfig
M src/cpu/x86/Makefile.inc
M src/include/rules.h
M src/include/symbols.h
4 files changed, 3 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/34806/1

diff --git a/src/cpu/Kconfig b/src/cpu/Kconfig
index 6b3f9a1..8b42199 100644
--- a/src/cpu/Kconfig
+++ b/src/cpu/Kconfig
@@ -6,15 +6,10 @@

if ARCH_X86

-config CACHE_AS_RAM
-	bool
-	default y
-
 config CAR_GLOBAL_MIGRATION
    bool
    default n
    default y if !RELOCATABLE_RAMSTAGE
-	depends on CACHE_AS_RAM
    help
      This option is selected if there is need to migrate CAR globals.
      All stages which use CAR globals can directly access the variables
diff --git a/src/cpu/x86/Makefile.inc b/src/cpu/x86/Makefile.inc
index 8a669bd..fef6f46 100644
--- a/src/cpu/x86/Makefile.inc
+++ b/src/cpu/x86/Makefile.inc
@@ -1,6 +1,6 @@
 ifeq ($(CONFIG_ARCH_ROMSTAGE_X86_32)$(CONFIG_ARCH_ROMSTAGE_X86_64),y)
 ifeq ($(CONFIG_CAR_GLOBAL_MIGRATION),y)
-romstage-$(CONFIG_CACHE_AS_RAM) += car.c
+romstage-y += car.c
 endif
 endif

diff --git a/src/include/rules.h b/src/include/rules.h
index d8f6e74..10cd715 100644
--- a/src/include/rules.h
+++ b/src/include/rules.h
@@ -284,7 +284,7 @@
 /* x86 specific. Indicates that the current stage is running with cache-as-ram
  * enabled from the beginning of the stage in C code. */
 #if defined(__PRE_RAM__)
-#define ENV_CACHE_AS_RAM CONFIG(CACHE_AS_RAM)
+#define ENV_CACHE_AS_RAM CONFIG(ARCH_X86)
 #else
 #define ENV_CACHE_AS_RAM 0
 #endif
diff --git a/src/include/symbols.h b/src/include/symbols.h
index cc27275..76c9320 100644
--- a/src/include/symbols.h
+++ b/src/include/symbols.h
@@ -72,7 +72,7 @@
  * (Does not necessarily mean that the memory is accessible.) */
 static inline int preram_symbols_available(void)
 {
-	return !CONFIG(CACHE_AS_RAM) || ENV_CACHE_AS_RAM;
+	return !CONFIG(ARCH_X86) || ENV_CACHE_AS_RAM;
 }

#endif /* __SYMBOLS_H */

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34806 )

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

Patch Set 1: Code-Review+2

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34806 )

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34806/1/src/cpu/x86/Makefile.inc 
File src/cpu/x86/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/34806/1/src/cpu/x86/Makefile.inc@3 
PS1, Line 3: y
romstage-$(CONFIG_CAR_GLOBAL_MIGRATION) += car.c

And nuke the guards?

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

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34806 )

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34806/1/src/cpu/x86/Makefile.inc 
File src/cpu/x86/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/34806/1/src/cpu/x86/Makefile.inc@3 
PS1, Line 3: y
> romstage-$(CONFIG_CAR_GLOBAL_MIGRATION) += car.c […]

File has following so I thought guard is required:

ROMSTAGE_CBMEM_INIT_HOOK(car_migrate_variables)

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

Hello Aaron Durbin, Arthur Heymans, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,

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

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

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

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

arch/x86: Obsolete CACHE_AS_RAM config

It was originally inverse of romcc -built romstages on x86,
and is currently always true on x86.

Change-Id: I65fa6b3ce8a86781724bbf08f5eadee4112667c4
Signed-off-by: Kyösti Mälkki 
---
M src/cpu/Kconfig
M src/cpu/x86/Makefile.inc
M src/include/rules.h
M src/include/symbols.h
4 files changed, 3 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/34806/2

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34806 )

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34806/1/src/cpu/x86/Makefile.inc 
File src/cpu/x86/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/34806/1/src/cpu/x86/Makefile.inc@3 
PS1, Line 3: y
> File has following so I thought guard is required: […]

line 2 is checking for y. Why wouldn't my suggestion be equivalent?

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

Hello Aaron Durbin, Arthur Heymans, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,

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

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

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

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

arch/x86: Obsolete CACHE_AS_RAM config

It was originally inverse of romcc -built romstages on x86,
and is currently always true on x86.

Change-Id: I65fa6b3ce8a86781724bbf08f5eadee4112667c4
Signed-off-by: Kyösti Mälkki 
---
M src/cpu/Kconfig
M src/cpu/x86/Makefile.inc
M src/include/rules.h
M src/include/symbols.h
4 files changed, 3 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/34806/3

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

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34806 )

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34806/1/src/cpu/x86/Makefile.inc 
File src/cpu/x86/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/34806/1/src/cpu/x86/Makefile.inc@3 
PS1, Line 3: y
> line 2 is checking for y. […]

Ahh.. I just read the 'nuke the guards' part of your first comment.

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34806 )

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34806/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/34806/1//COMMIT_MSG@9 
PS1, Line 9: romcc -built
no space

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34806 )

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

Patch Set 3: Code-Review+2

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

Hello Aaron Durbin, Arthur Heymans, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,

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

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

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

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

arch/x86: Obsolete CACHE_AS_RAM config

It was originally inverse of romcc-built romstages on x86,
and is currently always true on x86.

Change-Id: I65fa6b3ce8a86781724bbf08f5eadee4112667c4
Signed-off-by: Kyösti Mälkki 
---
M src/cpu/Kconfig
M src/cpu/x86/Makefile.inc
M src/include/rules.h
M src/include/symbols.h
4 files changed, 3 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/34806/4

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

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34806 )

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34806/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/34806/1//COMMIT_MSG@9 
PS1, Line 9: romcc -built
> no space

Done

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

Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34806 )

Change subject: arch/x86: Obsolete CACHE_AS_RAM config
......................................................................

arch/x86: Obsolete CACHE_AS_RAM config

It was originally inverse of romcc-built romstages on x86,
and is currently always true on x86.

Change-Id: I65fa6b3ce8a86781724bbf08f5eadee4112667c4
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34806
Tested-by: build bot (Jenkins) 
Reviewed-by: Aaron Durbin 
---
M src/cpu/Kconfig
M src/cpu/x86/Makefile.inc
M src/include/rules.h
M src/include/symbols.h
4 files changed, 3 insertions(+), 12 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Aaron Durbin: Looks good to me, approved

diff --git a/src/cpu/Kconfig b/src/cpu/Kconfig
index 645767c..69ceb64 100644
--- a/src/cpu/Kconfig
+++ b/src/cpu/Kconfig
@@ -6,14 +6,9 @@

if ARCH_X86

-config CACHE_AS_RAM
-	bool
-	default y
-
 config CAR_GLOBAL_MIGRATION
    bool
    default n
-	depends on CACHE_AS_RAM
    help
      This option is selected if there is need to migrate CAR globals.
      All stages which use CAR globals can directly access the variables
diff --git a/src/cpu/x86/Makefile.inc b/src/cpu/x86/Makefile.inc
index 8a669bd..9c18d44 100644
--- a/src/cpu/x86/Makefile.inc
+++ b/src/cpu/x86/Makefile.inc
@@ -1,8 +1,4 @@
-ifeq ($(CONFIG_ARCH_ROMSTAGE_X86_32)$(CONFIG_ARCH_ROMSTAGE_X86_64),y)
-ifeq ($(CONFIG_CAR_GLOBAL_MIGRATION),y)
-romstage-$(CONFIG_CACHE_AS_RAM) += car.c
-endif
-endif
+romstage-$(CONFIG_CAR_GLOBAL_MIGRATION) += car.c

subdirs-y += pae
 subdirs-$(CONFIG_PARALLEL_MP) += name
diff --git a/src/include/rules.h b/src/include/rules.h
index d8f6e74..10cd715 100644
--- a/src/include/rules.h
+++ b/src/include/rules.h
@@ -284,7 +284,7 @@
 /* x86 specific. Indicates that the current stage is running with cache-as-ram
  * enabled from the beginning of the stage in C code. */
 #if defined(__PRE_RAM__)
-#define ENV_CACHE_AS_RAM CONFIG(CACHE_AS_RAM)
+#define ENV_CACHE_AS_RAM CONFIG(ARCH_X86)
 #else
 #define ENV_CACHE_AS_RAM 0
 #endif
diff --git a/src/include/symbols.h b/src/include/symbols.h
index cc27275..76c9320 100644
--- a/src/include/symbols.h
+++ b/src/include/symbols.h
@@ -72,7 +72,7 @@
  * (Does not necessarily mean that the memory is accessible.) */
 static inline int preram_symbols_available(void)
 {
-	return !CONFIG(CACHE_AS_RAM) || ENV_CACHE_AS_RAM;
+	return !CONFIG(ARCH_X86) || ENV_CACHE_AS_RAM;
 }

#endif /* __SYMBOLS_H */