Change in coreboot[4.11_branch]: cpu/amd/car: Remove C source incldue of disable_cache_as_ram.c
Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48464 )
Change subject: cpu/amd/car: Remove C source incldue of disable_cache_as_ram.c ......................................................................
cpu/amd/car: Remove C source incldue of disable_cache_as_ram.c
Change-Id: Ib4841756127a3a28e400f4ad4bd58786dd343739 Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com --- M src/cpu/amd/car/disable_cache_as_ram.c M src/cpu/amd/car/post_cache_as_ram.c M src/cpu/amd/family_10h-family_15h/Makefile.inc M src/cpu/amd/family_10h-family_15h/init_cpus.c M src/include/cpu/amd/car.h M src/include/cpu/amd/model_10xxx_rev.h 6 files changed, 15 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/48464/1
diff --git a/src/cpu/amd/car/disable_cache_as_ram.c b/src/cpu/amd/car/disable_cache_as_ram.c index fb632bb..0ef50f9 100644 --- a/src/cpu/amd/car/disable_cache_as_ram.c +++ b/src/cpu/amd/car/disable_cache_as_ram.c @@ -15,10 +15,14 @@
#include <arch/cpu.h> #include <cpu/x86/cache.h> +#include <cpu/x86/mtrr.h> +#include <cpu/amd/mtrr.h> #include <cpu/x86/msr.h> #include <cpu/amd/msr.h> +#include <cpu/amd/car.h> +#include <cpu/amd/model_10xxx_rev.h>
-static __always_inline uint32_t amd_fam1x_cpu_family(void) +uint32_t amd_fam1x_cpu_family(void) { uint32_t family;
@@ -28,7 +32,6 @@ return family; }
-static __always_inline void disable_cache_as_ram_real(uint8_t skip_sharedc_config) { msr_t msr; diff --git a/src/cpu/amd/car/post_cache_as_ram.c b/src/cpu/amd/car/post_cache_as_ram.c index 78e417f..0eb8906 100644 --- a/src/cpu/amd/car/post_cache_as_ram.c +++ b/src/cpu/amd/car/post_cache_as_ram.c @@ -16,16 +16,16 @@ #include <arch/stages.h> #include <arch/early_variables.h> #include <commonlib/helpers.h> +#include <cpu/x86/cache.h> #include <cpu/x86/mtrr.h> #include <cpu/amd/mtrr.h> #include <cpu/amd/car.h> #include <cpu/amd/msr.h> +#include <cpu/amd/model_10xxx_rev.h> #include <arch/acpi.h> #include <program_loading.h> #include <romstage_handoff.h>
-#include "cpu/amd/car/disable_cache_as_ram.c" - // For set_sysinfo_in_ram() #include <northbridge/amd/amdfam10/raminit.h>
@@ -118,7 +118,7 @@ asmlinkage void cache_as_ram_new_stack(void) { print_car_debug("Disabling cache as RAM now\n"); - disable_cache_as_ram_real(0); // inline + disable_cache_as_ram_real(0);
disable_cache(); /* Enable cached access to RAM in the range 0M to CACHE_TMP_RAMTOP */ diff --git a/src/cpu/amd/family_10h-family_15h/Makefile.inc b/src/cpu/amd/family_10h-family_15h/Makefile.inc index f116042..2bc2abb 100644 --- a/src/cpu/amd/family_10h-family_15h/Makefile.inc +++ b/src/cpu/amd/family_10h-family_15h/Makefile.inc @@ -1,5 +1,6 @@ romstage-y += ../../x86/mtrr/earlymtrr.c romstage-y += ../car/post_cache_as_ram.c +romstage-y += ../car/disable_cache_as_ram.c
romstage-y += init_cpus.c romstage-y += fidvid.c diff --git a/src/cpu/amd/family_10h-family_15h/init_cpus.c b/src/cpu/amd/family_10h-family_15h/init_cpus.c index 61598d2..f56ee93 100644 --- a/src/cpu/amd/family_10h-family_15h/init_cpus.c +++ b/src/cpu/amd/family_10h-family_15h/init_cpus.c @@ -27,7 +27,9 @@ #include <northbridge/amd/amdht/AsPsDefs.h> #include <northbridge/amd/amdht/porting.h> #include <northbridge/amd/amdht/h3ncmn.h> +#include <cpu/amd/car.h> #include <cpu/amd/family_10h-family_15h/fidvid.h> +#include <cpu/amd/model_10xxx_rev.h> #include <southbridge/amd/common/reset.h>
#if CONFIG(SOUTHBRIDGE_AMD_SB700) @@ -38,8 +40,6 @@ #include <southbridge/amd/sb800/sb800.h> #endif
-#include "cpu/amd/car/disable_cache_as_ram.c" - #if CONFIG(PCI_IO_CFG_EXT) static void set_EnableCf8ExtCfg(void) { @@ -326,7 +326,7 @@ msr_t msr; uint32_t family;
- family = amd_fam1x_cpu_family(); // inline + family = amd_fam1x_cpu_family();
if (family < 0x6f) { /* Family 10h or earlier */ @@ -353,7 +353,7 @@ } }
- disable_cache_as_ram_real(skip_sharedc_config); // inline + disable_cache_as_ram_real(skip_sharedc_config);
/* Mark the core as sleeping */ lapic_write(LAPIC_MSG_REG, (apicid << 24) | F10_APSTATE_ASLEEP); diff --git a/src/include/cpu/amd/car.h b/src/include/cpu/amd/car.h index 359fa6b..c11ce7c 100644 --- a/src/include/cpu/amd/car.h +++ b/src/include/cpu/amd/car.h @@ -6,6 +6,7 @@ void cache_as_ram_main(unsigned long bist, unsigned long cpu_init_detectedx); asmlinkage void *post_cache_as_ram(void); asmlinkage void cache_as_ram_new_stack(void); +void disable_cache_as_ram_real(uint8_t skip_sharedc_config);
void disable_cache_as_ram(void);
diff --git a/src/include/cpu/amd/model_10xxx_rev.h b/src/include/cpu/amd/model_10xxx_rev.h index 88d395e..4b37baa 100644 --- a/src/include/cpu/amd/model_10xxx_rev.h +++ b/src/include/cpu/amd/model_10xxx_rev.h @@ -17,7 +17,7 @@ #define __CPU_AMD_MODEL_10XXX_REV_H__
int init_processor_name(void); - +uint32_t amd_fam1x_cpu_family(void); /* place holder for Family 10 revision code */
#endif /* __CPU_AMD_MODEL_10XXX_REV_H__ */
Hello Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48464
to look at the new patch set (#2).
Change subject: cpu/amd/car: Remove C source incldue of disable_cache_as_ram.c ......................................................................
cpu/amd/car: Remove C source incldue of disable_cache_as_ram.c
Change-Id: Ib4841756127a3a28e400f4ad4bd58786dd343739 Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com --- M src/cpu/amd/car/disable_cache_as_ram.c M src/cpu/amd/car/post_cache_as_ram.c M src/cpu/amd/family_10h-family_15h/Makefile.inc M src/cpu/amd/family_10h-family_15h/init_cpus.c M src/include/cpu/amd/car.h M src/include/cpu/amd/model_10xxx_rev.h 6 files changed, 15 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/48464/2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48464 )
Change subject: cpu/amd/car: Remove C source incldue of disable_cache_as_ram.c ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48464/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48464/3//COMMIT_MSG@7 PS3, Line 7: cpu/amd/car: Remove C source incldue of disable_cache_as_ram.c include
https://review.coreboot.org/c/coreboot/+/48464/3/src/cpu/amd/car/disable_cac... File src/cpu/amd/car/disable_cache_as_ram.c:
https://review.coreboot.org/c/coreboot/+/48464/3/src/cpu/amd/car/disable_cac... PS3, Line 31: static __always_inline I would guess the always_inline here is not accidental, but I did not trace this to its origins. The caller appears to be pretty much in the middle of CAR teardown, so there may have been a concern about the return address getting trashed with the entire stack in CAR.
If your target is to get fam15h back to 4.12+ you are probably going to rewrite a lot of this for POSTCAR_STAGE. Ask around what's needed before committing too much on little fixups like this. Also if you plan to support ACPI S3 suspend, to avoid low-memory corruptions (due the use of wbinvd vs invd) prepare for further complications here.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48464 )
Change subject: cpu/amd/car: Remove C source incldue of disable_cache_as_ram.c ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48464/3/src/cpu/amd/car/disable_cac... File src/cpu/amd/car/disable_cache_as_ram.c:
https://review.coreboot.org/c/coreboot/+/48464/3/src/cpu/amd/car/disable_cac... PS3, Line 31: static __always_inline
I would guess the always_inline here is not accidental, but I did not trace this to its origins. […]
You are mostly right and I am aware of the amount of work needed. Wanted to get rid of most C includes as possible. As you have said, these are little fixups and didn't cost me that much. If needed I will abandon it and address whole CAR when migrating to POSTCAR_STAGE
Michał Żygowski has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48464 )
Change subject: cpu/amd/car: Remove C source incldue of disable_cache_as_ram.c ......................................................................
Abandoned
-
Kyösti Mälkki (Code Review) -
Michał Żygowski (Code Review)